-
Notifications
You must be signed in to change notification settings - Fork 355
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
docs(menu-toggle): Adds docs for split button examples. #10268
Conversation
Preview: https://patternfly-react-pr-10268.surge.sh A11y report: https://patternfly-react-pr-10268-a11y.surge.sh |
@thatblindgeye lmk what you think about my initial edits / if I'm off the mark on things! |
|
||
``` | ||
|
||
### Split button toggle with checkbox and toggle button text | ||
### Split button toggle with checkbox and clickable text label |
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.
Maybe Split button toggle with a labelled checkbox
? "clickable text label" makes me think of the clickable toggle text use case more than the onChange
-triggering use case (and could be moved to line 210 for that example, but just having "text label" alone is also fine with me for 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.
The "...with labelled checkbox" could work. Maybe the other example name could be tweaked to "...with checkbox and toggle text label"? @edonehoo wdyt?
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.
I really like "a labelled checkbox"!
For the "clickable" example - I was trying to find a way to make things more concise/short and avoid writing "toggle" multiple times in the heading, but definitely want to be precise
- Is "split button toggle with checkbox label that toggles" accurate? If not, I can go with Eric's suggestion instead!
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.
Yeah having to repeat the "Split button toggle" text makes things difficult here 😆 Could "checkbox label that toggles" imply that the text is toggling the checkbox?
Do you think updating the examples to "Split toggle..." rather than "Split button toggle..." would help at all? I feel like ideally it'd be great to have a separate examples section for "Split button toggle examples" then the example titles could just omit the "Split button toggle" part, but I feel like there's still the issue with how our examples are rendered in Org that would cause some issue with 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.
"a separate examples section for "Split button toggle examples"" is exactly what I was wishing we could do 😭 but yeah I remember there being org restrictions around that
I like the idea of going with "split toggle". It's a small change, but it'll help cut down on wordiness, while still getting the point across!
I went with your previous selection "Split toggle with checkbox and toggle text label" & updated the other example headings in my last commit. I think I'm still having my own comprehension issues with these examples that get in the way of my suggestions lol - so as long as what we have makes sense from a dev standpoint I'm happy!
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.
To be fair the examples are very similar, so I definitely get any confusion or anything with them! Thanks for making these updates 🪨 ⭐
…0268) * docs(menu-toggle): Adds docs for split button examples. * Update names of split toggle examples.
…0268) * docs(menu-toggle): Adds docs for split button examples. * Update names of split toggle examples.
…0268) * docs(menu-toggle): Adds docs for split button examples. * Update names of split toggle examples.
…0268) * docs(menu-toggle): Adds docs for split button examples. * Update names of split toggle examples.
* docs(Page): add example showing different type prop variants (#10352) * docs(Page): add example showing different type prop variants * docs(Page): make text color white in "subnav" section * docs(Page): rename IDs in example * docs(Table): update column management example to use link button for Cancel action (#10459) * feat(Modal): support ReactNode type for 'title' prop * fix(DatePicker/CalendarMonth): better range styling when some dates are disabled (#10398) * fix(DatePicker/CalendarMonth): range styling when disabled dates * chore(Datepicker): getElementSelectorToFocus function description * feat(MenuToggle): add OUIA support * docs(menu-toggle): Adds docs for split button examples. (#10268) * docs(menu-toggle): Adds docs for split button examples. * Update names of split toggle examples. * Add description list basic demo (#10184) * add description list basic demo * fix DashboardWrapper not found issue * implement review comments * add divider and size * update content * fixed layout issues * Convert demo application to Vite * Upgrade Testing Library dependencies to latest version * upgrade to latest version * upgrade to latest version * upgrade to latest version * chore(deps): replace camel-case with change-case * Masthead: add demo that includes horizontal nav * chore: use protocol for workspace dependencies * chore(deps): upgrade to the latest version * chore(deps): upgrade to the latest version * chore(deps): upgrade to the latest version * chore(deps): remove unused dependency * chore(deps): upgrade to the latest version * chore(deps): ugprade Jest to latest version * chore(deps): upgrade to the latest version * chore(deps): upgrade dependencies to the latest version * chore(deps): upgrade to the latest version * chore(deps): update dependencies * update yarn file * more deps fixes * more deps * fix(build): get our custom TS transformers working again (#10477) * fix(build): get our custom TS transformers working again * chore(deps): Update lock file * Update lock file * chore(deps): Update yarn lock * fix demo app * cherry pick yarn changes * update yarn.lock * updatw snapshots * fix demo app * feat(templates): toggle props & improvements (#10473) * feat(templates): toggle props & improvements * remove toggleContent from typeahead template * update template names * update tests * added SimpleSelect tests * fix yarnlock * fix(CodeEditor) support Mac and PC shortcuts in example (#10472) * updatee lock file --------- Co-authored-by: adamviktora <84135613+adamviktora@users.noreply.github.com> Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com> Co-authored-by: Ajay Pratap <ajay.pratap233@gmail.com> Co-authored-by: Austin Sullivan <ausulliv@redhat.com> Co-authored-by: kmcfaul <45077788+kmcfaul@users.noreply.github.com> Co-authored-by: Dominik Petřík <77832970+Dominik-Petrik@users.noreply.github.com>
Closes #10258