POC docs: <uxdot-pattern> support media query patterns with an iframe viewport#3005
POC docs: <uxdot-pattern> support media query patterns with an iframe viewport#3005zeroedin wants to merge 21 commits into
<uxdot-pattern> support media query patterns with an iframe viewport#3005Conversation
Adds a `viewport` boolean attribute to uxdot-pattern that renders the pattern in a standalone iframe instead of inline shadow DOM. This lets patterns that rely on @media queries respond to their own resizable container rather than the parent page viewport. - SSR controller generates a full HTML page with importmap and global styles, written to a _viewport/ subdirectory - Client controller hydrates the iframe src from SSR'd markup - Iframe is resizable via CSS resize: both - Empty-description grid rules scoped to non-viewport mode to prevent code-tabs overlapping the iframe Made-with: Cursor
✅ Deploy Preview for red-hat-design-system ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Review Summary by QodoAdd iframe viewport mode and stacked navigation pattern to uxdot-pattern
WalkthroughsDescription• Add viewport attribute to uxdot-pattern for iframe-based pattern rendering with @media query support • Create uxdot-context-select component replacing rh-context-picker with improved mobile UX • Implement stacked navigation pattern with sticky scroll and z-index coordination • Add viewport size controls (mobile/tablet/desktop) for responsive pattern preview Diagramflowchart LR
A["uxdot-pattern<br/>viewport attribute"] -->|"generates"| B["SSR Controller<br/>writes iframe HTML"]
B -->|"creates"| C["_iframe/<br/>pattern.html"]
C -->|"renders in"| D["iframe element<br/>with resize"]
A -->|"uses"| E["uxdot-context-select<br/>color picker"]
E -->|"posts message"| D
F["Stacked Navigation<br/>Pattern"] -->|"uses"| A
F -->|"demonstrates"| G["scroll-state()<br/>+ IntersectionObserver"]
File Changes1. uxdot/uxdot-pattern.ts
|
Code Review by Qodo
1.
|
|
Size Change: +2.86 kB (+0.91%) Total Size: 317 kB 📦 View Changed
ℹ️ View Unchanged
|
<uxdot-pattern> support media query patterns with an iframe viewport
Documentation Health
|
| Category | Score | |
|---|---|---|
| Element description | 0/25 | ❌ |
| Attribute documentation | 5/20 | ❌ |
| Slot documentation | 0/15 | ❌ |
| CSS documentation | 2/15 | ❌ |
| Event documentation | 15/15 | ✅ |
| Demos | 0/10 | ❌ |
ColorPaletteListConverter — 65/90 ⚠️
| Category | Score | |
|---|---|---|
| Element description | 0/25 | ❌ |
| Attribute documentation | 20/20 | ✅ |
| Slot documentation | 15/15 | ✅ |
| CSS documentation | 15/15 | ✅ |
| Event documentation | 15/15 | ✅ |
| Demos | 0/0 | ❌ |
paletteMap — 65/90 ⚠️
| Category | Score | |
|---|---|---|
| Element description | 0/25 | ❌ |
| Attribute documentation | 20/20 | ✅ |
| Slot documentation | 15/15 | ✅ |
| CSS documentation | 15/15 | ✅ |
| Event documentation | 15/15 | ✅ |
| Demos | 0/0 | ❌ |
paletteNames — 65/90 ⚠️
| Category | Score | |
|---|---|---|
| Element description | 0/25 | ❌ |
| Attribute documentation | 20/20 | ✅ |
| Slot documentation | 15/15 | ✅ |
| CSS documentation | 15/15 | ✅ |
| Event documentation | 15/15 | ✅ |
| Demos | 0/0 | ❌ |
ContextChangeEvent — 65/90 ⚠️
| Category | Score | |
|---|---|---|
| Element description | 0/25 | ❌ |
| Attribute documentation | 20/20 | ✅ |
| Slot documentation | 15/15 | ✅ |
| CSS documentation | 15/15 | ✅ |
| Event documentation | 15/15 | ✅ |
| Demos | 0/0 | ❌ |
rh-context-picker (RhContextPicker) — 52/100 ⚠️
| Category | Score | |
|---|---|---|
| Element description | 0/25 | ❌ |
| Attribute documentation | 12/20 | |
| Slot documentation | 15/15 | ✅ |
| CSS documentation | 10/15 | |
| Event documentation | 15/15 | ✅ |
| Demos | 0/10 | ❌ |
Recommendations:
- rh-context-demo: add descriptions to all attributes (Attribute documentation, +10 pts)
- rh-context-demo: add at least one demo with a URL (Demos, +5 pts)
- rh-context-picker: add a description to explain what this declaration does (Element description, +5 pts)
- rh-context-demo: use RFC 2119 keywords (MUST, SHOULD, AVOID) to clarify requirements (Element description, +5 pts)
- rh-context-picker: add at least one demo with a URL (Demos, +5 pts)
adamjohnson
left a comment
There was a problem hiding this comment.
This is a pretty neat idea. There's still work to do around design refinement and fallback browser support, but cool to see.
There was a problem hiding this comment.
This will need an accompanying image in docs/assets/patterns/ for the Stacked Navigation thumbnail on the All Patterns page.
| } | ||
|
|
||
| select { | ||
| appearance: none; |
There was a problem hiding this comment.
This is what this looks like in Safari (browser without base-select support yet. We definitely wanna add a caret icon for the non-custom select supporting browsers. See #2959 for and example of how to do that.
| <rh-surface id="content">${allContent}</rh-surface> | ||
| <div id="controls-row"> | ||
| ${this.viewport ? html` | ||
| <rh-button-group id="viewport-controls" |
There was a problem hiding this comment.
I feel like these buttons could use some further design refinement. Perhaps some styling to button group to help unify them, which may include equalizing their height to the neighboring select.
Might add a title attr/tooltip too.
| <rh-surface id="content">${allContent}</rh-surface> | ||
| <div id="controls-row"> | ||
| ${this.viewport ? html` | ||
| <rh-button-group id="viewport-controls" |
| const label = this.accessibleLabel | ||
| || this.#internals.computedLabelText | ||
| || 'Color palette'; | ||
| return this.hasUpdated ? html` |
There was a problem hiding this comment.
@bennypowers What do you think about this pattern? This is essentially a workaround to SSR a custom select.
Currently, Lit SSR and <selectedcontent> don't play nice together.
If you deem this acceptable, I can apply this pattern to <rh-scheme-dropdown>.
There was a problem hiding this comment.
i need to understand more what the issues are with litssr and selectedcontent. we'll need to have a show and tell i think
generally though, if there's state affecting render which changes on the clientside after SSR, that render has to happen only after hasUpdated
There was a problem hiding this comment.
Here's a little backstory:
There was a problem hiding this comment.
thanks for bringing this up
go with the multiple render paths solution for now, gated on hasUpdated
There was a problem hiding this comment.
with a refactor in 42158ab this is now in rh-context-picker.ts
|
@coreyvickery and @marionnegp I'd love your feedback on this proof of concept. Not necessarily for a critique of what I've done but to get your wheels spinning for how you can take this idea and improve upon it. |
<uxdot-pattern> support media query patterns with an iframe viewport<uxdot-pattern> support media query patterns with an iframe viewport
THIS IS A Proof of Concept
This is meant for initial discussion, likely needing design input and future direction
What I did
viewportattribute touxdot-patternwhich encapsulates the pattern in an iframeuxdot-context-select, heavily borrowing from @adamjohnson work on therh-scheme-dropdown.rh-context-pickercomponent making the UI more mobile friendly. I expect if we like this we can release it asrh-context-selectbut those are exported in the package so kept this named uxdot-* for the POC.Testing Instructions
Notes to Reviewers
This exposes
rh-navigation-secondaryinability to respond to systemlight | darkmode. Known issue we should likely fix.