Skip to content

Latest commit

 

History

History
38 lines (33 loc) · 14.4 KB

File metadata and controls

38 lines (33 loc) · 14.4 KB

TODO

  • Add Test Coverage to Playwright Tests if Possible
  • Some of the notes in this "Filterable/Searchable combobox" section below really need to be Notes or Development Notes. Please update them as soon as possible to reduce the clutter here without losing any of the important information on design decisions. Feel free to open a Design Decisions file if you feel it's necessary as well.
  • When do we want to consider a searchable Combobox -- if ever? Should it be its own component? Or should the ComboboxField simply be configurable?
    • Inform developers that ComboboxOption.filteredOut IS NOT to be used whenever one pleases. Instead, it allows developers to override how the ComboboxField marks options as filtered out. This property should only be toggled/read while the ComboboxField is actively resetting its filter or performing its filtering logic (whether with its own implementation or with a developer's overriding implementation).
    • Document the difference between required and unclearable/clearable clearly.
    • Document why we always attach handleSearch as a listener, and right at the beginning. (Wanting to make sure we can call event.stopImmediatePropagation in time.) EDIT: I think this is irrelevant now.
    • Document that developers who don't want to show a "No Options" message to their end users can simply hide the entire listbox with CSS if all options are [data-filtered-out] (i.e., [role="listbox"]:not(:has([role="option"]:not([data-filtered-out])))).
    • If deemed necessary, document why we went with an attribute like filter/search instead of using a standardized attribute (like aria-autocomplete) directly. (We don't know if we'll support the other variant of aria-autocomplete yet. It's too weird/complicated/confusing for consumers and maybe even maintainers to have a mount-only attribute that triggers another attribute which triggers another attribute (filter --> aria-autocomplete --> contenteditable). If we want to support other versions of aria-autocomplete in the future, we can just update the "universally owning attribute" (e.g., filter --> filter-type). filter is arguably part of the API, where as the other 2 attributes are implementation details. (We could technically just use ElementInternals.MyAriaAttribute.) We don't want to mess up the JS-free <select> instance by encouraging users to use standardized props like autocomplete either.)
    • Document why we will use <input>/<div> instead of modifying text directly inside <combobox-field>. (Basically, Shadow DOM support for Ranges and Selections is sketchy across browsers, sadly. So it's safer to just use an <input> that's in a Shadow DOM. If our suggestion gets accepted, then we'll have an advantage with this change in the future anyway... Actually, we went back to using a contenteditable <combobox-field> because we concluded that both the filterable and non-filterable versions of our component should have a consistent UX to avoid confusion. So we don't need an easy way to move the cursor around. HOWEVER, we later realized that contenteditable="plaintext-only" elements can still include newline characters! They're basically <textarea>s! That's HUGE no-no for us. And it's far more complicated to try to combat that than it is to use a regular <input> which doesn't support newline characters. So now we're back to using <input>/<div>. We don't like the idea of switching element types as the filter attribute is updated, but we don't have a choice. We played with having an input[readonly] so that we didn't have to switch elements, but it seems to risk confusing Screen Readers, and it sounds like some browsers apply default styles. Unfortunately... if we go this route... we'll have to figure out transfering attributes appropriately onmount... there might be a way that isn't complicated?)
      • Note: Before you go crazy and think about trying to use beforeinput so that we can still stick to modifying the text content of the <combobox-field> during "Filter Mode", note that taking this approach will aggressively move the input cursor to the beginning of the text field (though only during value updates). This is a bad UX on top of a weird maintenance decision. Just use the correct HTMLElement (<input>) for the job of searching and stop debating this topic. EDIT: We debated the topic again. lol. We actually found that beforeinput is needed to remove a lot of confusion regardless of whether we're using <combobox-field contenteditable="true"> or <input>. There are reasons for that which we'll explain later. Unfortunately, it seems <input> might have better a11y support/clarity than a contenteditable element, so we'll probably switch to <div>/<input> in the future. But using an editable <combobox-field> is a far better Maintainer Experience and Developer Experience. We'll see if there's any way we can stick to just contenteditable, but probably won't work out. EDIT x2: We forgot that, as pointed out in our 2nd note, we want our tool to be compatible with the FormValidityObserver, so we might need to keep contenteditable after all... Maybe we can update the FormValidityObserver to work even if we use an inner <input>? Or maybe there's a way to get Screen Readers to view contenteditable elements more accurately... (NOTE: It seems that the filterable combobox experience using contenteditable is significantly more normal when using VoiceOver with Safari. Things get a little weird with Chrome, and they get very weird with Firefox. In fact, some things are weird in Firefox even just for regular form controls/labels when it comes to VoiceOver. In any case, it might not be the end of the world if we use contenteditable after all... But there will likely be mixed support for Screen Readers. We should probably test tools besides VoiceOver, though hopefully users can still get the jist of what's happening when interacting with our contenteditable element.)
    • Document why put ComboboxField.formControl in Shadow DOM instead of Light DOM. (Putting in Shadow DOM saves users from having their wrapping <form> elements from picking up "noise" by seeing an irrelevant <input> element when the ComboboxField is filterable. Unfortunately, this puts us at a disadvantage because we can't style based on [role="combobox] anymore. So we'll have to use ElementInternals.states instead... NOT ideal AT ALL... But we're kinda forced into this functionality. We can still do our best to indicate ARIA attributes in our roles and states. Hmmmm... How does that affect stuff like styling for :focus, :hover, and the like, though? ... Maybe we should transfer all CSS styles to the inner element? EDIT: Actually, if we want to avoid the "Form Noise" problem, we can just set form="" on the related internal form controls as needed and leave our stuff in the Light DOM. This might be more convenient since it could let us reuse our existing styles... in fact, this approach will be NECESSARY if for ANY reason we need to apply the primary styles on a parent element that hosts the Shadow DOM element. For example, if we put the border styles on <select-enhance> to account for potential Icons that could appear within the component -- like a Caret Icon -- then we would have to update the border styles based on the :hover/:focus status of the ComboboxField.formControl... but you can't access Shadow DOM parts with something like combobox-field:has(combobox-field::part(form-control):state(my-state))! So that locks us into a specific approach, basically... unless perhaps :focus-within works (unlikely)... actually, :focus-within works so maybe we're good? ... Eh... Actually, we still wouldn't be able to style based on things like [aria-invalid="true"] ... We could try to keep track of when the field is invalid from within our own code and perhaps apply aria-invalid to our internal form control accordingly and then expose something like :states(aria-invalid)...but the problem with that approach is that it doesn't handle use cases where the user wants to supply aria-invalid to the form control directly and then apply styles accordingly... Yes, they can technically access ComboboxField.formControl to apply the aria-invalid role -- yes, it belongs on the item with role="combobox", not necessarily on the <combobox-field> element -- but that gets hacky and inconvenient pretty quickly, don't you think? Or maybe people listening for events on the ComboboxField would already have to do that for aria-invalid anyway? Man... this is hard... Actually, never mind. Scrap ALL OF IT. We want our component to be compatible with the FormValidityObserver, and that basically requires the <combobox-field> to be the element that contains the ARIA attributes and everything else. So we're going to have to go back to contenteditable and do some fancy logic to prevent newlines from appearing. Additionally, maybe we can use beforeinput to prevent input from being captured or bubbled? 🤔)
    • Document that users need to leverage the white-space (or white-space-collapse) CSS Property if they want users to see when they've entered multiple whitespaces into the filter. (Note that this is a very important CSS style to apply. If the white-space CSS property collapses all whitespace, the spaces will still exist in the user's physical search value, thus disrupting the displayed filtered options. This would certainly result in an unintuitive UX.)
    • Document why we may support custom filering logic for filterable comboboxes but not not regular ones. (For typeahead functionality, any logic besides startsWith would result not only in an experience that's inconsistent with <select>, but also an experience that's incredibly confusing for Users — perceivably. It would be hard to know where the active option would jump next since you can jump to any option that vaguely matches the current search string. This is different from when you're in filter mode: The user can always see and easily modify any part of their filter at any time. And within a few keystrokes, it becomes obvious what the rules are for filtering.)
    • Document internally why we default to startsWith instead of includes in filter mode.
    • Document the order in which the Combobox Web Component parts must be registered/defined.
  • Do we want to add/support a Caret Icon for the Combobox component?
  • Add CSS for select-enhancer > select (for when JS is disabled/unavailable).
  • Make a note about using different kinds of Combobox "Adapters"/"Wrappers".
  • Make a note that the value of <combobox-option> (and therefore a <select>'s <option>) MUST be unique (for accessibility reasons related to aria-activedescendant and HTML's disallowing of duplicate ids). This shouldn't realistically cause problems for anyone. I don't know if duplicate values have a valid use case anyway. Duplicate values will produce unpredictable behavior.
  • Add documentation in general about how this component works, what expectations are, and what feature parity is with native <select>
    • DEFINITELY don't forget to document how delegated event listeners for input will get a little wild. (Regular delegated event listeners should be fine. But for captured event listeners, people will have to check things like !event.isTrusted or something similar which would indicate that the event actually originated from <combobox-field> instead of <input>.)
  • Make YouTube video(s) documenting how to augment the TypeScript element/JSX types in the popular JS Frameworks. This will be helpful for us and for any others who try to accomplish what we did. And it'll save everyone a lot of time.
  • Should our tests assume that our Combobox Component is in a <form> by default since the custom element itself is a Form Control?
  • We should definitely add a test proving that our component works in Shadow DOMs at some future point.
  • Open a GitHub Issue that addresses the various issues with ElementInternals.validity and ElementInternals.setValidity(). See our Notes for more details.
  • Consider adding a GitHub CI Action to lint our code.
  • Unless we're mistaken, Playwright currently has a bug. For some reason, the tests related to tabbing are failing for Playwright's WebKit browser. However, tabbing works fine manually in Safari. Since it works manually, we can investigate this more later or open a Playwright bug. (It's probably a Playwright bug since we didn't actually change any of our code surrounding focusing/tabbing -- unless this is an Operating System issue.) NOTE: For right now, the tests still seem to be passing on CI. So this really seems to be a Playwright issue and/or an OS issue (or some other similar, inconspicuous issue).

Potential Considerations

  • Don't the native <option> elements allow safe attribute/property changes even if they aren't connected to the DOM? Should we allow something similar? (Mainly thinking of supporting changing the ComboboxOption.selected property in isolation.) For example, maybe we could support this by returning early based on !this.combobox? Need to investigate... and evaluate if this is even worthwhile...
  • Far-off thought: Should we give developers an easy way to set this component up themselves? We'd also want to make sure that frameworks can create our Web Component just fine, without running into any problems. (There would only be a concern if people wanted to provide <combobox-option> and <combobox-field> directly instead of using <select-enhancer> in conjunction with <select> -- at least, that's our assumption). Note: This ties into our idea about "Adapters", and it might be sufficient to delegate this problem to UserLand with the "Adapters" concept.

IMMEDIATE CURRENT WORK/PRIORITY

Double check our existing code to make sure it is [sufficiently] coherent and not absolute garbage before you go adding new features (like the formResetCallback support).