GAUD-9843 - Skeleton of the panel mixin and the three components#6928
Conversation
…real, sticky componets with hardcoded z-indexes
This reverts commit 551d5c7.
Co-authored-by: Dave Lockhart <dlockhart@users.noreply.github.com>
…65-Flesh-out-demo-page # Conflicts: # components/page/demo/page-component.js
…65-Flesh-out-demo-page
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
# Conflicts: # components/page/demo/page-component.js # components/page/page.js
| .header { | ||
| position: relative; | ||
| z-index: 15; /* To be over sticky content of our core components */ | ||
| } | ||
|
|
||
| .page.header-sticky .header { | ||
| position: sticky; | ||
| top: 0; | ||
| z-index: 15; /* To be over sticky content of our core components */ | ||
| } |
There was a problem hiding this comment.
Instead of this, we could implement one of the strategies @dbatiste describes in this comment, making the panel headers only apply sticky once the navbar has scrolled way. I'm not sure if it's "worth it" to add all that until we run into a problem with this, but also flexible with what others think.
There was a problem hiding this comment.
I think the stuff added in the headers for these demos could be a bit better, but I've run out of time 😅
| this._slotVisibility = {}; | ||
| } | ||
|
|
||
| _handleSlotVisibilityChange(e) { |
There was a problem hiding this comment.
This I think can be #handleSlotVisibilityChange?
| } | ||
|
|
||
| #handleDialogOpen() { | ||
| this.shadowRoot.querySelector('#demo-dialog').opened = true; |
There was a problem hiding this comment.
Could have a reactive state _demoDialogOpened that binds to the ?opened of the dialog.
|
|
||
| static styles = [pagePanelStyles, css` | ||
| .panel-header { | ||
| top: var(--d2l-page-header-height, 0); |
There was a problem hiding this comment.
Could also set a variable that the mixin uses to avoid these subclasses knowing much about the internals of the base class.
There was a problem hiding this comment.
Not sure what you mean? Like set a different css property with this value like --d2l-page-panel-top-offset?
There was a problem hiding this comment.
Yeah, or just --d2l-page-panel-top, and this would set that on its :host. Not a big deal.
There was a problem hiding this comment.
I need to adjust this/add a few more uses and variables to fix the sticky stuff (next draft PR going up), so we can figure it out there. It's a bit confusing and I don't love what I have
|
🎉 This PR is included in version 3.243.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
This adds the three panel components (
d2l-page-main,d2l-page-side-nav, andd2l-page-supporting). Not tied to this being a mixin versus shared styles and helper functions. I think it's kinda hard to see right now which is best when they're so simple.