-
Notifications
You must be signed in to change notification settings - Fork 590
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
Pro 4440 - accessible/keyboard-able relationship field autocompletes #4728
Pro 4440 - accessible/keyboard-able relationship field autocompletes #4728
Conversation
…440/accessible-keyboardable-relationship-autocompletes
this.searchSuggestion = !qs.autocomplete && this.suggestion; | ||
this.searchHint = (!qs.autocomplete || !results.length) && this.hint; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Suggestion and Hint UI objects were originally part of the list of results. I separated these out so that I could cleanly pseudo focus search suggestions by index and not have to worry about accounting for the fake UI 'results'
this.handleFocusOut(); | ||
this.input(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flash UI to remove select element from list
if (!this.searchList.length) { | ||
this.input(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trigger suggestions if they are hidden
if (!this.searchList.length) { | ||
this.input(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trigger suggestions if they are hidden
This field needs to be updated with a more traditional multi-select combo box UI rather than appending the slats underneath, it makes for an awkward structure when making multiple selections |
modules/@apostrophecms/schema/ui/apos/logic/AposInputRelationship.js
Outdated
Show resolved
Hide resolved
if (this.searchList.length) { | ||
// psuedo focus first element | ||
this.searchFocusIndex = 0; | ||
} | ||
}, | ||
handleFocusOut() { | ||
// hide search list when click outside the input | ||
// timeout to execute "@select" method before | ||
setTimeout(() => { | ||
this.searchList = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we reset this.searchSuggestion
and this.searchHint
too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think its necessary .. those props are refreshed every search and the UI is closed every selection (when it is reopened search is run again), so I don't think the get out of sync
Co-authored-by: haroun <[email protected]>
…etes' of github.com:apostrophecms/apostrophe into pro-4440/accessible-keyboardable-relationship-autocompletes
UI Tests have passed here @haroun https://github.com/apostrophecms/testbed/actions/runs/11467364290 |
Summary
https://linear.app/apostrophecms/issue/PRO-4440/%5Baccessibility%5D-make-relationship-field-suggestions-accessible
What are the specific steps to test this change?
relationshjip.mp4
What kind of change does this PR introduce?
(Check at least one)
Make sure the PR fulfills these requirements:
If adding a new feature without an already open issue, it's best to open a feature request issue first and wait for approval before working on it.
Other information: