-
Notifications
You must be signed in to change notification settings - Fork 29
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
Collapsible Sidebar #317
Collapsible Sidebar #317
Conversation
…zif-Khan/appsody-website into feature-bk-collapsibleSidebar
…zif-Khan/appsody-website into feature-bk-collapsibleSidebar
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.
Two issues that jump out to me, but nothing huge:
On iPhone 8 and 8 Plus size classes, the hover state causes the elements to rearrange. This doesn't look great and does affect the usability as when you try to click a link, it jumps around. While theres no hover states on mobile, a narrow window on desktop will show mobile views.
Also, the font increasing in size is causing the sidebar elements to jump around too as the size increase causes the items to increase in height. This could be fixed with a fixed height or something similar, or removing the size change and just showing the chevron and the colour change.
…zif-Khan/appsody-website into feature-bk-collapsibleSidebar
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.
All looking good now!
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
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.
All good!
Checklist
fixes: #299