Skip to content
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

Merged
merged 2 commits into from
Jun 3, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,9 @@ import { MenuToggle } from '@patternfly/react-core';

### Split button toggle with checkbox

To add an action button or other control to a menu toggle, use a split button. A `<MenuToggle>` can be rendered as a split button by adding a `splitButtonOptions` object. Elements to be displayed before the toggle button must be included in the `items` property of `splitButtonOptions`.
To add a checkbox (or other action/control) to a menu toggle, use a split button.

A `<MenuToggle>` can be rendered as a split button by adding a `splitButtonOptions` object. Elements to be displayed before the toggle button must be included in the `items` property of `splitButtonOptions`.

The following example shows a split button with a `<MenuToggleCheckbox>`.

Expand All @@ -205,21 +207,21 @@ Variant styling can be applied to split button toggles to adjust their appearanc

```

### Split button toggle with checkbox label
### Split button toggle with checkbox and text label

To display text in a split button menu toggle, add a label to the `items` property of `splitButtonOptions`.
To add a text label to a split button toggle, pass `children` to the `<MenuToggle>` component.

```ts file='MenuToggleSplitButtonCheckboxWithText.tsx'
```ts file='MenuToggleSplitButtonCheckboxWithToggleText.tsx'

```

### Split button toggle with checkbox and toggle button text
### Split button toggle with checkbox and clickable text label
Copy link
Contributor

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).

Copy link
Contributor

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?

Copy link
Contributor Author

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!

Copy link
Contributor

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.

Copy link
Contributor Author

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!

Copy link
Contributor

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 🪨 ⭐


For split button toggles that should still contain text which will trigger the toggle's `onClick`, pass `children` to the `MenuToggle`.
You can allow users to select a toggle checkbox by clicking either the checkbox or the text label.

The following example shows a split button with a `<MenuToggleCheckbox>` and toggle button text.
To do so, pass `children` to the `<MenuToggleCheckbox>` component. When the menu toggle text is clicked, the checkbox's `onChange` callback will be triggered.

```ts file='MenuToggleSplitButtonCheckboxWithToggleText.tsx'
```ts file='MenuToggleSplitButtonCheckboxWithText.tsx'

```

Expand Down
Loading