Add option to auto navigate to the first item on open#81
Conversation
src/index.ts
Outdated
| Array.from(this.list.querySelectorAll<HTMLElement>('[role="option"]:not([aria-disabled="true"])')) | ||
| .filter(visible)[0] | ||
| ?.setAttribute('data-combobox-option-default', 'true') | ||
| } |
There was a problem hiding this comment.
Would it work to add an else if (this.firstOptionSelectionMode === 'focused') {this.navigate(1)} case here instead of defining a separate focusDefaultOptionIfNeeded method? That way we only have to call one function in start and we can't forget to add the second case if we call indicateDefaultOption elsewhere.
There was a problem hiding this comment.
I opt'ed for a new method because this method was also being called here:
Line 136 in c28985b
There was a problem hiding this comment.
ooh I see. I think I still would rather combine the methods, but then maybe here we add a conditional if (this.firstOptionSelectionMode === 'active') (or if (this.firstOptionSelectionMode !== 'selected')?)
iansan5653
left a comment
There was a problem hiding this comment.
Looks great! Just a documentation suggestion
There was a problem hiding this comment.
Oh! We should also update the docs in the README - I just realized they describe the settings as well. We could probably use the same text as in the proposed comment.
Co-authored-by: Ian Sanders <iansan5653@github.com>
This is an extension to the
firstDefaultOptionprop, and infact, a replacement, but extending an additional mode to this to now allow for the following three behaviours as a config prop:firstDefaultOptionis true).This PR does the following:
firstDefaultOptionboolean with a type config of three optionsMore than happy to update the naming if people feel strongly here.