-
Notifications
You must be signed in to change notification settings - Fork 0
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
Sticky header hide and show on scroll and visible login access #72
Conversation
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.
This pull request does not contain a valid label. Please add one of the following labels: ['type: feature', 'type: change', 'type: fix', 'type: removal', 'target: developer-experience', 'type: internal']
decidim-core/app/views/layouts/decidim/header/_menu_breadcrumb_main_dropdown.html.erb
Outdated
Show resolved
Hide resolved
decidim-core/app/views/layouts/decidim/header/_menu_breadcrumb_main_dropdown.html.erb
Outdated
Show resolved
Hide resolved
decidim-core/app/views/layouts/decidim/header/_main_links_mobile_search.html.erb
Outdated
Show resolved
Hide resolved
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 @ElviaBth I've left you some comment to solve the duplicated IDs
decidim-core/app/views/layouts/decidim/header/_mobile_language_choose.html.erb
Outdated
Show resolved
Hide resolved
decidim-core/app/views/layouts/decidim/header/_mobile_language_choose.html.erb
Outdated
Show resolved
Hide resolved
decidim-core/app/views/layouts/decidim/header/_menu_breadcrumb_main_dropdown.html.erb
Outdated
Show resolved
Hide resolved
decidim-core/app/views/layouts/decidim/header/_menu_breadcrumb_main_dropdown.html.erb
Outdated
Show resolved
Hide resolved
decidim-core/app/views/layouts/decidim/header/_menu_form_search_mobile.html.erb
Show resolved
Hide resolved
decidim-core/app/views/layouts/decidim/header/_menu_form_search_mobile.html.erb
Show resolved
Hide resolved
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 @ElviaBth! I found I couple of things missing when reviewing this:
- On mobile I don't see the switch of the logo to the favicon
- When I log in as admin, I don't see the avatar and the access to the user menu
- When I log in as a participant with no avatar associated, I don't see the avatar by default and the access to the user menu
- When I log in as a participant, I do see the avatar and the user menu, but I miss the indicator of pending notifications/conversations, this one 👇
The language selector and the search inside the hamburger menu work perfect!
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! We've checked again and we are almost there! A couple of last adjustments:
-
The margin between menu and avatar gets misaligned if the avatar has no image. I'm attaching screenshots for comparison (the correct margin is like the one with image):
-
On the other hand, the notification indicator should be displayed as in the avatar example without image, otherwise it is too far apart.
-
As we are using the initial when the avatar has no image, we should apply it here as well.
🎩 What? Why?
Sticky header hide and show on scroll and visible login access
📌 Related Issues
Testing
You can visit with a mobile browser.
📷 Screenshots
header default
header login photo
add search added to the menu and language chooser to dropdown menu