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

feat(templates): toggle props & improvements #10473

Merged
merged 6 commits into from
Jun 20, 2024
Merged

feat(templates): toggle props & improvements #10473

merged 6 commits into from
Jun 20, 2024

Conversation

kmcfaul
Copy link
Contributor

@kmcfaul kmcfaul commented May 29, 2024 •
edited
Loading

What: Closes #10409

  • Adds toggleProps (and toggleWidth if it was missing) to templates.
  • Extends main prop interface with respective component (SelectProps for Select and DropdownProps for Dropdown).
  • Ensures that onToggle prop is called on every open state change.
  • Removes default ids, ids can be passed manually if necessary already and this way multiple templates on a page won't require ids to avoid a11y issues.
  • Adds description to some examples.

Open questions:
Do we want to rename SelectSimple -> SimpleSelect, SelectTypeahead -> TypeaheadSelect, DropdownSimple -> SimpleDropdown to better align with CheckboxSelect?

@patternfly-build
Copy link
Contributor

patternfly-build commented May 29, 2024 •
edited
Loading

Preview: https://patternfly-react-pr-10473.surge.sh

A11y report: https://patternfly-react-pr-10473-a11y.surge.sh

@kmcfaul kmcfaul linked an issue May 29, 2024 that may be closed by this pull request
Revisit completed templates for API consistency/refinement #10267
Closed
wise-king-sullyman
wise-king-sullyman reviewed Jun 3, 2024
View reviewed changes
Copy link
Contributor

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

One request, I know that we have a followup issue for adding test suites for the others, but can you update the test suites for DropdownSimple and CheckboxSelect to include those new props since those test suites are otherwise already complete?

Also I would be on board with that name change.

@tlabaj tlabaj requested a review from nicolethoen June 3, 2024 20:36
@tlabaj
Copy link
Contributor

tlabaj commented Jun 3, 2024

Name change sounds good

thatblindgeye
thatblindgeye approved these changes Jun 10, 2024
View reviewed changes
Copy link
Contributor

@thatblindgeye thatblindgeye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also agreed on the name change. Other than Austin's comment about tests lgtm

@nicolethoen
Copy link
Contributor

@kmcfaul this needs a rebase

@kmcfaul kmcfaul force-pushed the template-improvements branch from 580eaf8 to 3e2bf68 Compare June 12, 2024 18:15
wise-king-sullyman
wise-king-sullyman requested changes Jun 14, 2024
View reviewed changes
Copy link
Contributor

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the test suites for SimpleDropdown and CheckboxSelect to include those new props since those test suites are otherwise already complete? I'd really love those to not regress in test completeness.

@kmcfaul kmcfaul force-pushed the template-improvements branch from b7990c9 to 174cf49 Compare June 20, 2024 17:13
@kmcfaul
Copy link
Contributor Author

kmcfaul commented Jun 20, 2024 •
edited
Loading

@wise-king-sullyman Updated the Checkbox/Dropdown test and also added tests for SimpleSelect

I'm planning on adding tests to TypeaheadSelect / any other test updates during this sprint to try to close out the templates epic.

wise-king-sullyman
wise-king-sullyman approved these changes Jun 20, 2024
View reviewed changes
Copy link
Contributor

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥳

@wise-king-sullyman wise-king-sullyman merged commit 315fc80 into patternfly:main Jun 20, 2024
13 checks passed
@patternfly-build
Copy link
Contributor

Your changes have been released in:

  • @patternfly/react-docs@6.4.0-prerelease.26
  • @patternfly/react-templates@1.1.0-prerelease.17

Thanks for your contribution! 🎉

tlabaj pushed a commit to tlabaj/patternfly-react that referenced this pull request Jun 25, 2024
@kmcfaul @tlabaj
feat(templates): toggle props & improvements (patternfly#10473)
695f0be 
* feat(templates): toggle props & improvements

* remove toggleContent from typeahead template

* update template names

* update tests

* added SimpleSelect tests

* fix yarnlock
gitdallas pushed a commit that referenced this pull request Jun 25, 2024
@tlabaj @adamviktora
@edonehoo @ajaypratap003 @wise-king-sullyman @kmcfaul @Dominik-Petrik
Cherry pick main to v6 (#10619)
9e7b462 
* 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>
kmcfaul added a commit to kmcfaul/patternfly-react that referenced this pull request Jun 27, 2024
@kmcfaul
feat(templates): toggle props & improvements (patternfly#10473)
efd321e 
* feat(templates): toggle props & improvements

* remove toggleContent from typeahead template

* update template names

* update tests

* added SimpleSelect tests

* fix yarnlock
dlabaj pushed a commit that referenced this pull request Jul 2, 2024
@kmcfaul @adamviktora @thatblindgeye
feat(templates): patch release for templates (#10676)
f68cf02 
* feat(Select): Typeahead template (#10235)

* fix(SelectTypeahead example): make "no results" option aria-disabled

* fix(SelectTypeahead example): don't close the menu on input click when there is text

* fix(SelectTypeahead example): remove visual focus on item after closing the menu

Prevents situation where we open the menu via focusing on the toggle arrow and clicking enter -- then two items can have focus styling, which is not ideal.

* fix(SelectTypeahead example): remove check icon from the selected option when input text changes

* fix(SelectTypeahead example): rename example

* feat(Select): add prop to opt out of focusing first menu item on open

Flag prop shouldFocusFirstMenuItemOnOpen has been added, because of typeahead select, which should keep focus on the input.

* refactor(SelectTypeahead example): adaption on first menu item focused

* feat(MenuToggle): make typeahead toggle button not focusable

* fix(SelectTypeahead example): focus input after toggle button click

* feat(SelectTypeahead example): change the focused item on hover

* fix(SelectTypeahead example): don't focus on first item after tabbing

* feat(Select): add typeahead select template

* fix(SelectTypeahead): address PR review

- new changes were done also based on SelectTypeahead example updates (#10207)

* fix(SelectTypeahead template): call onToggle every time menu opens/closes

* refactor(SelectTypeahead template)

* feat(Select): Typeahead example (#10207)

* refactor(Select): rename shouldFocusFirstMenuItemOnOpen

* feat(SelectTypeahead example): better arrow up/down keys handling

- does not apply visual focus on the first menu option
- handles disabled options
- opens menu on pressing up/down arrow keys

* feat(SelectTypeahead example): don't close menu on clicking clear button when open

* refactor(SelectTypeahead example)

* refactor(SelectTypeahead example)

* fix(SelectTypeaheadCreatable example): changes based on SelectTypeahead

* fix(SelectMultiTypeahead example): changes based on SelectTypeahead

* fix(SelectTypeaheadCreatable example): don't show create option if that exact option exists

* fix(SelectMultiTypeaheadCreatable): changes based on SelectTypeahead

* fix(SelectMultiTypeaheadCheckbox): changes based on SelectTypeahead

* fix(SelectTypeaheadCreatable): close menu after creating option

* fix(SelectTypeahead template): rename prop back to shouldFocusFirstItemOnOpen

* feat(Dropdown): Added simple template (#10308)

* feat(Dropdown): Added simple template

* Added tests

* Added imports to example file

* Additional fixes for docs fail

* Updated import name

* Added additional tests

* 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(): update demo-app version and snap

---------

Co-authored-by: adamviktora <84135613+adamviktora@users.noreply.github.com>
Co-authored-by: Eric Olkowski <70952936+thatblindgeye@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thatblindgeye thatblindgeye thatblindgeye approved these changes

@nicolethoen nicolethoen nicolethoen approved these changes

@wise-king-sullyman wise-king-sullyman wise-king-sullyman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Templates - select improvements Revisit completed templates for API consistency/refinement
6 participants
@kmcfaul @patternfly-build @tlabaj @nicolethoen @wise-king-sullyman @thatblindgeye