-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Meet the team e2e test #1090
Meet the team e2e test #1090
Conversation
@boodland is attempting to deploy a commit to the Chayn Team on Vercel. A member of the Team first needs to authorize it. |
@kyleecodes a follow up issue/ticket can be created to add tests for the rest of languages and complete the fixture with the rest of the supporting team members, I did not include all of them to save time and be able to move to another migration task, also they can be good first tasks for new contributors. Thanks in advance |
6ff4767
to
a147035
Compare
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -127,7 +128,7 @@ const StoryblokTeamMemberCard = (props: StoryblokTeamMemberCardProps) => { | |||
</CardActionArea> | |||
<Collapse in={expanded} timeout="auto" unmountOnExit> | |||
<CardContent sx={collapseContentStyle}> | |||
<Typography variant="body2" mb={0} paragraph> | |||
<Typography variant="body2" mb={0} paragraph component={'div'}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@boodland - this was originally set as a paragraph component. Is there a reason why you wanted to set it to div?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @eleanorreem, semantically it is not allowed to have a p tag inside another p tag. React hydratation is throwing an error due to that, I have set to a div as the text content is in the deeper p tag that is created by the render function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes total sense now. I see what the issue is - richText is being rendered inside a paragraph. Not ideal! I'd just remove the "paragraph" prop as well.
@boodland thanks for these tests. Super thorough - one small question on reassigning a typography component to a div! |
9aa9929
to
a5ab0aa
Compare
Bloom frontend Run #667
Run Properties:
|
Project |
Bloom frontend
|
Branch Review |
develop
|
Run status |
Failed #667
|
Run duration | 06m 52s |
Commit |
44e9907a4c: Meet the team e2e test (#1090)
|
Committer | Alberto Arias |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
3
|
Flaky |
0
|
Pending |
2
|
Skipped |
0
|
Passing |
38
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
Tests for review
Brill - thanks @boodland ! |
Issue link / number:
#1089
What changes did you make?
Created meet-the-team e2e test for english version
Why did you make the changes?
So the page is covered with test and help us to validate it is working when making changes or migrating it to app router
Did you run tests?
Yes