feat(Dropdown): Added simple template #10308
Conversation
|
Preview: https://patternfly-react-pr-10308.surge.sh A11y report: https://patternfly-react-pr-10308-a11y.surge.sh |
thatblindgeye
force-pushed
the
dropdownTemplate
branch
from
5c86ba6
to
2afbc3d
Compare
thatblindgeye
force-pushed
the
dropdownTemplate
branch
from
5d90302
to
c96acf4
Compare
There was a problem hiding this comment.
LGTM
For the name - we started with SelectSimple, but we moved to CheckboxSelect/TypeaheadSelect for other templates. I was wondering if we overall wanted to keep the {X}Simple naming pattern for the basic Select/Dropdown templates, or rename them to Simple{X} or just {X} as the import path is different?
Not a blocker, just curious if anyone felt strongly one way or the other.
|
@kmcfaul I kind of want to suggest "[X]Template[template type]", but depending on how many we actually end up with over time that could get verbose. SelectTemplateSimple, SelectTemplateTypeahead, SelectTemplateMultiTypeahead... Maybe something like that would work better if the API was more At the very least I think there should be some quantifier attached to the component name. Even though the import path is different, and a consumer could differentiate the import names if they're using both composable and template Select, could just make it easier to outright name them differently. I've probably gotten used to having the component name start names from our examples/demos, e.g. |
There was a problem hiding this comment.
I like your suggestion, @thatblindgeye. I feel like "[X]Template[template type]" makes it clearer. I could see it getting pretty verbose like you mentioned though.
* feat(Dropdown): Added simple template * Added tests * Added imports to example file * Additional fixes for docs fail * Updated import name * Added additional tests
* 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>
What: Closes #10303
Additional issues: