-
Notifications
You must be signed in to change notification settings - Fork 153
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
[Mobile Nav] Added sliding effect and allowed multiple open accordions #12260
Conversation
ec8775d
to
9128e57
Compare
a907754
to
409d914
Compare
402621d
to
45762ae
Compare
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.
Desktop
- When moving from one item to another, I think there is a quick fade/ease out that makes it look like there's a bit of background flashing through going on (this could be simplified)
Tablet
- Line below navigation disappears after opening item like “who we are” or after when other items snap to top
- Border reappears when closing but items do not reset so top item (so “who we are” gets hidden) appears fully hidden
- The snap to top is working inconsistently for me, but mainly “Blog” moves up a bit more all the time while the others often stay in place which makes it feel inconsistent/unpredictable
Mobile
- When closing a section other than the first section, a strip is seen of “who we are” but does not fully reset back to the top
I’m thinking overall, having close/open animation without snap to top would provide a more predictable/consistent experience
Mobile
|
The tablet/mobile issues resolved with removing the snap to top combo. Wanted to check in on desktop, if it was possible to not have the background flashing through when transitioning from one primary nav item to another? Video comparing PR to figma mockup to demonstrate what I mean: |
557c3af
to
432b499
Compare
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.
LGTM 🚀. And very clear comments 📝, thanks Mavis!
➤ Simon Acosta Torres commented: PR has already been merged. |
Description
Link to review app: https://foundation-s-feature-ma-36qyhj.herokuapp.com/en/ (CMS cred:
admin2
/admin2password
)Related PRs/issues: #12233
┆Issue is synchronized with this Jira Story