- Issue created by @saschaeggi
- @saschaeggi opened merge request.
- Status changed to Needs review
about 1 year ago 11:07pm 7 October 2023 - 🇪🇸Spain ckrina Barcelona
I love this! For me this is working fine, but as I mentioned in Slack I recall Claire finding a bug when she was trying to do the same. Let's wait until Monday so she can get a chance to review this after working on 📌 Scroll the sidebar when it's collapsed Fixed .
On the other side I see this is assuming the logo needs to stay sticky. The way the toolbar is implemented right now wouldn’t need it (the logo is not that important) but we might:
- end up moving the collapse button to the top. We're exploring designs after the feedback from 🌱 Toolbar Prototype Usability Testing Phase 2 Fixed .
- end up opening that top region in the future to place stuff in there.
So for now the logo at the top sticky would be a change on behavior/designs although we might end up needing in the future. For this reason, I'd move the conversation about the logo sticky into a follow-up, and if Claire doesn't have any feedback from this we can merge the bottom one.
- 🇨ðŸ‡Switzerland saschaeggi Zurich
@ckrina as this uses a sticky background technique the only issue I could think of would have been that the scrim was not shown when a background color is used like on the active subnavigation. But I've worked around this issue which using an
rgba()
value. Another method would be to usemix-blend-mode
butrgba()
might be the nicer option here.On the other side I see this is assuming the logo needs to stay sticky. The way the toolbar is implemented right now wouldn’t need it (the logo is not that important) but we might:
Not at all. Just the bare minimum changes here (to move the scoll area to the content area) make it appear sticky, but we can change that as well.
So for now the logo at the top sticky would be a change on behavior/designs although we might end up needing in the future. For this reason, I'd move the conversation about the logo sticky into a follow-up, and if Claire doesn't have any feedback from this we can merge the bottom one.
We can change this behavior but would need to restructure the markup a little bit. I did not want to touch that just yet. Agreed that this could be done in a follow-up.
- 🇨🇦Canada claireristow
I also love this, thanks @saschaeggi! The CSS solution is cleaner and much less buggy.
The shadow itself looks a bit different but that can be adjusted in a follow-up issue if needed. Looks good to merge on my end!
- 🇨ðŸ‡Switzerland saschaeggi Zurich
@ckrina rebased this MR, ready 🎉
-
ckrina →
committed f78f7efa on 1.x authored by
saschaeggi →
Issue #3392407: Use CSS only scrolling shadows (scrim)
-
ckrina →
committed f78f7efa on 1.x authored by
saschaeggi →
- Status changed to Fixed
about 1 year ago 5:07pm 10 October 2023 - 🇪🇸Spain ckrina Barcelona
Ok, merging this but a couple of follow-ups need to be addressed:
- As mentioned, the header/logo shouldn't be sticky on desktop.
- The most important one: the sticky region shouldn't be sticky on mobile. It doesn't give enough room to interact with the menu and is a regression on a usability perspective (specially if we want to test it next week).
Automatically closed - issue fixed for 2 weeks with no activity.