Standarize side panels styles #13763
Conversation
github-actions
bot
added
APP: Facility
Build Artifacts
|
nucleogenesis
left a comment
nucleogenesis
left a comment
There was a problem hiding this comment.
Code changes look great overall, I added some minor notes, but this is good to go to QA.
One possible enhancement might be to consolidate shared styles into a sass file that is shared across the three side panels (or otherwise DRYing things up) - but that's def non-blocking.
| return { | ||
| /* Will be calculated in mounted() as it will get the height of the fixedHeader then */ | ||
| // @type {RefImpl<number>} | ||
| handleScroll, |
There was a problem hiding this comment.
The comments above this line belong to the windowBreakpoint beneath it
| .side-panel-content { | ||
| position: relative; | ||
| .side-panel-title { | ||
| font-size: 18px; |
There was a problem hiding this comment.
Yes, but for the acceptance criteria in the issue I added:
All headers are h1s and consistent (smaller, 18px) size. note: for 0.18 the header size was 18px; so we are favoring that for consistency even though on this figma spec it says 20px
There was a problem hiding this comment.
Thanks for catching that :)
|
Thanks @AlexVelezLl LGTM! |
|
Hey @nucleogenesis! I hadn't done the styles shared file initially because I was afraid of these styles being non-scoped just for the components that imported them, but I found a way to achieve this using scss mixins. Let me know what you think! |
nucleogenesis
left a comment
nucleogenesis
left a comment
There was a problem hiding this comment.
I gave it another quick test after the styles update and everything looks great, thanks @AlexVelezLl !
4 participants
Summary
All of them now shows a box shadow when the content is scrolled:
References
Closes #13731.
Reviewer guidance