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

lib: Remove custom key navigation from TypeaheadSelect #21412

Closed

Conversation

mvollmer
Copy link
Member

@mvollmer mvollmer commented Dec 9, 2024

It's not helping.

The TypeaheadSelect template code will focus the text input 100ms
after opening the menu. The tests generally aren't disturbed about
that, except when simulating key presses, of course.

When the focus unexpectedly shifts back to the text input while
navigating the menu items, the focused item is reset to either the
first or last one.
@mvollmer mvollmer force-pushed the lib-no-custom-menu-keynav branch from 6f2f191 to 20b87ec Compare December 9, 2024 12:01
Comment on lines +34 to +35
function add(val: string | number) {
setSelected(selected.concat([val]));
Copy link
Contributor

Choose a reason for hiding this comment

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

These 2 added lines are not executed by any test.

Comment on lines +38 to +39
function rem(val: string | number) {
setSelected(selected.filter(v => v != val));
Copy link
Contributor

Choose a reason for hiding this comment

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

These 2 added lines are not executed by any test.

id='multi-typeahead-widget'
placeholder="Select flavors"
isScrollable
noOptionsFoundMessage={notFoundIsString ? "Not found" : val => cockpit.format("'$0' not found", val) }
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test.

Comment on lines +53 to +54
onToggle={() => setToggles(val => val + 1)}
onInputChange={() => setChanges(val => val + 1)}
Copy link
Contributor

Choose a reason for hiding this comment

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

These 2 added lines are not executed by any test.

id="notFoundIsStringMulti"
label="notFoundIsString"
isChecked={notFoundIsString}
onChange={(_event, checked) => setNotFoundIsString(checked)}
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test.

@mvollmer
Copy link
Member Author

mvollmer commented Dec 9, 2024

This only works with the PF coming in #21399. Before that, the text input keeps its focus and the template does indeed handle all key presses for the menu navigation.

@mvollmer mvollmer closed this Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants