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.

Sign up for GitHub

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

Jump to bottom

MenuToggle split button doc updates #10258

Open
thatblindgeye opened this issue Apr 8, 2024 · 2 comments · May be fixed by #10268
Open

MenuToggle split button doc updates #10258

thatblindgeye opened this issue Apr 8, 2024 · 2 comments · May be fixed by #10268
Labels
Content Tech debt improvements to code that do not affect either user or product developers’ experience.
Milestone
2024.Q2

Comments

@thatblindgeye
Copy link
Contributor

Describe the enhancement or change
Follow up to #10256 (review)

We should update the "Split button toggle with checkbox label" example description to convey something more along the lines that in order to have a label for the checkbox of a split button toggle, consumers should pass children to MenuToggleCheckbox, and that clicking this text will trigger the checkbox's onChange callback.

Additionally we should update the children prop description to indicate how children is rendered for a split toggle.

cc @edonehoo if you wouldn't mind taking a look at the example titles/descriptions touched in the above linked PR

Is this request originating from a Red Hat product team? If so, which ones and is there any sort of deadline for this enhancement?

Any other information?
@thatblindgeye thatblindgeye added the Tech debt improvements to code that do not affect either user or product developers’ experience. label Apr 8, 2024
@edonehoo
Copy link
Contributor

edonehoo commented Apr 9, 2024

@thatblindgeye to make sure I'm understanding, is this correct?

  • the "split button toggle with checkbox label" example allows users to click anywhere in the toggle to select the checkbox?
  • Then the "split button toggle with checkbox and toggle button text" example requires the checkbox specifically to be clicked?
  • The implementation difference is whether or not the label is placed within the MenuToggleCheckbox?

For some reason it almost feels like the descriptions are opposite of the examples in that pr preview, so I just want to make sure I'm following 100% 😁

@thatblindgeye
Copy link
Contributor Author

@edonehoo Basically yeah. For both of those examples, there's the "checkbox area' and the "toggle button" area. So for the "with checkbox label" example, clicking the checkbox or the visible text should trigger the checkbox while clicking the caret icon will trigger the toggle button. For the "with checkbox and toggle button text" example it's the opposite where like you said only clicking the checkbox itself triggers it while clicking the text will trigger the toggle button.

@tlabaj tlabaj added the Content label Apr 10, 2024
@tlabaj tlabaj added this to the Prioritized Backlog milestone Apr 10, 2024
@edonehoo edonehoo linked a pull request Apr 10, 2024 that will close this issue
Open
@tlabaj tlabaj modified the milestones: Prioritized Backlog, 2024.Q2 May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Content Tech debt improvements to code that do not affect either user or product developers’ experience.
Projects
PatternFly Issues
Status: Backlog
Milestone
2024.Q2
Development

Successfully merging a pull request may close this issue.

docs(menu-toggle): Adds docs for split button examples. edonehoo/patternfly-react
3 participants
@tlabaj @thatblindgeye @edonehoo