-
Notifications
You must be signed in to change notification settings - Fork 33
feat(navigation-drawer): add <rh-navigation-drawer>
#2443
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
base: main
Are you sure you want to change the base?
Conversation
…2325) * fix(tag): improve color contrast on gray tags in dark schemes * docs(tag): improve slotted SVG color contrast on dark schemes * docs(tag): fix `grey` vs `gray` in alt text * chore(tag): add changeset
* feat(breadcrumb): add ability to truncate middle breadcrumb items This code was developed with possible assistance by generative AI. * chore(breadcrumb): add changeset * fix(breadcrumb): only truncate when `truncate` attr is present This code was developed with possible assistance by generative AI. * test(breadcrumb): fix `handleEvent` uncaught TypeError This code was developed with possible assistance by generative AI. * docs(breadcrumb): update changeset prose Co-authored-by: Benny Powers - עם ישראל חי! <[email protected]> * refactor(breadcrumb): rename truncate lightdom classes * refactor(breadcrumb): nest `[hidden]` under `rh-breadcrumb` selector * fix(breadcrumb): implement `render()` method for `truncate` functionality This code was developed with possible assistance by generative AI. * refactor: nest lightdom css * refactor: minor nitpicks * fix(breadcrumb): add `type` attr to truncate button * fix(breadcrumb): underline truncation button / ellipsis * fix(breadcrumb): use logical properties * lint(breadcrumb): css * test(breadcrumb): add tests for `truncate` This code was developed with possible assistance by generative AI. * test(breadcrumb): unify test examples * test(breadcrumb): add null guards --------- Co-authored-by: Benny Powers - עם ישראל חי! <[email protected]> Co-authored-by: Benny Powers <[email protected]>
* feat(code-block): add attribute/ability to hide line numbers * chore(code-block): add changeset * style(code-block): lint typescript and css * fix(code-block): change attr to `line-numbers` in place of `hide-line-numbers` * refactor(code-block): rename `lineNumbers` variable * chore(code-block): update changeset example * fix(code-block): prevent possible SSR mismatch errors * docs(code-block): add `line-numbers="hidden"` docs
* fix(card): allow all six color palettes on card
* docs(card): allow all color palettes in nested color context demo
* docs(card): add terminology clarifying color palette usage vs backgrounds
* docs(card): use `darkest` color palette with Promo demo as docs instruct
* chore(card): add changeset
* style(card): lint css
* fix(card): correct `color-palette` / card background behavior ("Option 3")
* docs(card): color context prose
* style(card): lint css
* docs(card): move color palette docs to style page
* fix(card): use `--rh-color-surface` for both `.light` and `.dark`
* refactor(card): surface colors for card background when `color-palette` is set
* refactor(card): move `computedPalette` to `willUpdate()`
* refactor(card): move promo vars to render method
* feat(code-block): reduce line numbers load * feat(code-block): adding return to computeLineNumbers * docs: changeset --------- Co-authored-by: Benny Powers - עם ישראל חי! <[email protected]>
* fix(code-block): fix syntax error breaking build * chore(code-block): add changeset * style: revert remove dom This is kind of a six-of-one scenario. is it better to remove DOM? or is it better to avoid thrashing the DOM when we add and remove on changes? In either case, I think It's more readable this way, even if there's an empty, aria-hidden dom node * chore: delete .changeset/shy-coins-fall.md --------- Co-authored-by: Benny Powers <[email protected]> Co-authored-by: Benny Powers - עם ישראל חי! <[email protected]>
* fix(tag): remove errant curly brace from demo * fix(tag): use logical properties * fix(tag): update interactive (link) tag styles * style(tag): alphabetize CSS selectors * fix(tag): make focus and active state include icon * feat(tag): add functionality for disabled linked tags * docs(tag): rename label 👉 tag in api docs * docs(tag): remove docs for `close` nonexistent event * docs(tag): rename demo to `links.html` * feat(tag): add ability to make a tag a `<button>` * feat(tag): FACE + click event for button tags * docs(tag): rename buttons demo * docs(tag): add changeset * docs(tag): update bi-modal `overview.svg` * docs(tag): update Style page * docs(tag): update guidelines page * docs(tag): update a11y page images * fix(tag): remove button variant See #2407 (comment) for more info. * fix(tag): allow clicks to pass through active state * fix(tag): keep `cursor: pointer;` for the active state on linked tags * docs(tag): remove button tag info from docs * fix(tag): remove button callback and event Co-authored-by: Benny Powers - עם ישראל חי! <[email protected]> * chore(tag): update changeset * fix(tag): prevent disabled linked tags from navigating to a URL --------- Co-authored-by: marionnegp <[email protected]> Co-authored-by: Benny Powers - עם ישראל חי! <[email protected]>
✅ Deploy Preview for red-hat-design-system ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: 4201cb8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Size Change: +3.43 kB (+1.48%) Total Size: 235 kB
ℹ️ View Unchanged
|
<rh-navigation-drawer><rh-navigation-drawer>
|
maybe this should be called |
I referenced this question in the issue before I saw your comment here. My thought is that |
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.
Few nitpicks. Overall, great work as always!
* fix(footer): remove logger warnings, use ifDefined * chore(footer): add changeset
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.
Still needs docs. Otherwise, LGTM!
|
@adamjohnson @zeroedin Can this be closed without docs? They'll be done next week. |
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.
We can't have multiple different drawer elements. see my comments on the issue.
|
Notes from chat:
Open questions and review note
Counterpoint: material 3 has a navigation drawer all in all, I think it's better that we put this feature back in the oven for more design, and shoot for a duoduo release cc @markcaron |
|
@bennypowers Moving to Doduo would give me more time to finish other things for Diglett/understand this element more so I can eventually write the docs. cc @markcaron |
|
Expanding on what Benny already mentioned. The container attribute was intended to target an ancestor not explicitly owned by the component. This is necessary for the resize observer to set the "compact" state switching to a disclosure for when the targeted container is not the direct parent of the component, but a higher ancestor in a complex layout. It is a code smell and not ideal. My thought at the time of writing was trying to navigate the possibility of a nested container-type causing problems in the ancestor tree. Furthermore, the demo isn't complete enough to fully demonstrate its real need. For the attribute to be truly effective, the We may have to take a more layout-based approach as @bennypowers mentioned vs the self-contained approach taken so far: vs layout-based where the drawer has a page content slot itself and then becomes the container: Likely, special care will have to be taken to avoid CLS of the whole page in this setup. |
What I did
<rh-navigation-drawer>Closes #2465
Related issue: #1329
Testing Instructions
Notes to Reviewers