josefarias / hotwire_combobox

An accessible autocomplete for Ruby on Rails.
https://hotwirecombobox.com
MIT License
456 stars 30 forks source link

Option is not selected after clicking on a substring match #93

Closed aroth closed 6 months ago

aroth commented 6 months ago

click

Clicking an option that does not startWith the input text results in no selection.

  Combobox.Selection = (Base) =>
    class extends Base {
      selectOptionOnClick(event) {
        this.filter(event);
        this._select(event.currentTarget); // <-- ISSUE HERE; pass in { forceAutocomplete: true } for desired functionality
        this.close();
      }

Selecting results the keyboard (hw-combobox #navigate) works just fine, just not on click.

josefarias commented 6 months ago

@aroth thanks for the clear example and diagnosis! Will get this sorted today.

josefarias commented 6 months ago

Oh, strange. I thought this would be easy to replicate but I seem to be missing something.

In https://hotwirecombobox.com/, if I go to the basic combobox example and type "iss" I get two matches: Mississippi and Missouri. Clicking any of those gets me the correct selection.

I'll keep looking. But, in the meantime, am I missing something obvious @aroth?

aroth commented 6 months ago

@josefarias I noticed this for async comboboxes. Our normal comboboxes are working mostly alright, with some other weirdness that I will follow up with. The async search examples on your homepage don't return results when searching in the middle of a word or string, so I couldn't confirm there. Knowing that, can you take a closer look?

With two comboboxes side by side, the async one (left) does not work while the sync one (right) does not. Gif below:

1111

I thought I had traced it here to the function below. force is false, and (startsWith(autocompletedValue, typedValue) also returns false, hence the issue. And yet the sync version still works, so 🤔.

      _autocompleteWith(option, { force }) {
        if (!this._autocompletesInline && !force) return;

        const typedValue = this._typedQuery;
        const autocompletedValue = option.getAttribute(
          this.autocompletableAttributeValue
        );

        if (force) {
          this._fullQuery = autocompletedValue;
          this._actingCombobox.setSelectionRange(
            autocompletedValue.length,
            autocompletedValue.length
          );
        } else if (startsWith(autocompletedValue, typedValue)) {
          this._fullQuery = autocompletedValue;
          this._actingCombobox.setSelectionRange(
            typedValue.length,
            autocompletedValue.length
          );
        }
      }

Network response for the async result:

<turbo-stream action="update" target="mission_supervisor_id-hw-listbox"><template>
    <li id="5d666d53-585e-4816-b412-e54acd29e19c" role="option" class="hw-combobox__option" data-action="click-&gt;hw-combobox#selectOptionOnClick" data-filterable-as="Burns, Cory" data-autocompletable-as="Burns, Cory" data-value="530">Burns, Cory</li>
</template></turbo-stream>
<turbo-stream action="remove" target="mission_supervisor_id__hw_combobox_pagination__wrapper"></turbo-stream>
<turbo-stream action="append" target="mission_supervisor_id-hw-listbox"><template>
  <!-- BEGIN /Users/aroth/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/hotwire_combobox-0.1.40/app/views/hotwire_combobox/_pagination.html.erb -->
<li id="mission_supervisor_id__hw_combobox_pagination__wrapper" data-hw-combobox-target="endOfOptionsStream" data-input-type="undefined" aria-hidden="true">
  <turbo-frame loading="lazy" id="mission_supervisor_id__hw_combobox_pagination"></turbo-frame>
</li><!-- END /Users/aroth/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/hotwire_combobox-0.1.40/app/views/hotwire_combobox/_pagination.html.erb -->
</template></turbo-stream>
josefarias commented 6 months ago

@aroth this is incredibly helpful thanks!

I've managed to replicate on the docs site by matching on the middle of the string – but only on the youtube example and not the basic async example with the states. That's enough though! I'll keep looking and hopefully have a fix soon. I'm sure you're right about the cause but I want to make sure I understand the bug completely.

Our normal comboboxes are working mostly alright, with some other weirdness that I will follow up with

Please do! I have full intent to have everything working perfectly under normal conditions so you can expect me to be responsive on these reports. Thanks!

josefarias commented 6 months ago

@aroth are you by chance using the name_when_new option on the async combobox that's not working?

Would you mind sharing the part of your view template where you render the combobox, please?

aroth commented 6 months ago

@josefarias Of course:

<div class="<%= container_class %>">
  <legend class="flex items-center gap-1 whitespace-nowrap">
    <%= form.label attribute, label_text, label_options %>
    <small>(Type to Search)</small>
  </legend>
  <%= form.combobox attribute, url, id: id_attribute, name_when_new: "#{form.object_name}[#{attribute}]" %>
</div>

I have followed-up with another issue re: ordering/selection we had also discovered.

aroth commented 6 months ago

Full HTML:

<fieldset class="hw-combobox" data-async-id="mission_supervisor_id" data-controller="hw-combobox" data-hw-combobox-expanded-value="true" data-hw-combobox-name-when-new-value="mission[supervisor_id]" data-hw-combobox-original-name-value="mission[supervisor_id]" data-hw-combobox-autocomplete-value="both" data-hw-combobox-small-viewport-max-width-value="640px" data-hw-combobox-async-src-value="/mission_form/supervisors?intent%5Ballow_unsupervised%5D=false&amp;intent%5Ballowed_supervisor_roles%5D%5B%5D=instructor&amp;intent%5Bname%5D=Initial+Qualification+Training&amp;for_id=mission_supervisor_id&amp;format=turbo_stream" data-hw-combobox-filterable-attribute-value="data-filterable-as" data-hw-combobox-autocompletable-attribute-value="data-autocompletable-as" data-hw-combobox-selected-class="hw-combobox__option--selected" data-hw-combobox-invalid-class="hw-combobox__input--invalid">
  <!-- BEGIN /Users/aroth/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/hotwire_combobox-0.1.40/app/views/hotwire_combobox/combobox/_hidden_field.html.erb -->
<input type="hidden" name="mission[supervisor_id]" id="mission_supervisor_id-hw-hidden-field" data-hw-combobox-target="hiddenField" autocomplete="off" value="cory">
<!-- END /Users/aroth/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/hotwire_combobox-0.1.40/app/views/hotwire_combobox/combobox/_hidden_field.html.erb -->
  <!-- BEGIN /Users/aroth/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/hotwire_combobox-0.1.40/app/views/hotwire_combobox/combobox/_input.html.erb --><input id="mission_supervisor_id" role="combobox" class="hw-combobox__input" type="text" data-action="focus->hw-combobox#open input->hw-combobox#filter keydown->hw-combobox#navigate click@window->hw-combobox#closeOnClickOutside focusin@window->hw-combobox#closeOnFocusOutside turbo:before-stream-render@document->hw-combobox#rerouteListboxStreamToDialog" data-hw-combobox-target="combobox" data-async-id="mission_supervisor_id" aria-controls="mission_supervisor_id-hw-listbox" aria-owns="mission_supervisor_id-hw-listbox" aria-haspopup="listbox" aria-autocomplete="both" aria-activedescendant="" autocomplete="off" aria-expanded="true">
<span class="hw-combobox__handle" data-action="click->hw-combobox#toggle" data-hw-combobox-target="handle"></span>
<!-- END /Users/aroth/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/hotwire_combobox-0.1.40/app/views/hotwire_combobox/combobox/_input.html.erb -->
  <!-- BEGIN /Users/aroth/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/hotwire_combobox-0.1.40/app/views/hotwire_combobox/combobox/_paginated_listbox.html.erb --><ul id="mission_supervisor_id-hw-listbox" role="listbox" class="hw-combobox__listbox" data-hw-combobox-target="listbox">
    <li id="eaf75b33-4f95-47d8-ba69-4b5409e2835c" role="option" class="hw-combobox__option" data-action="click->hw-combobox#selectOptionOnClick" data-filterable-as="Burns, Cory" data-autocompletable-as="Burns, Cory" data-value="530">Burns, Cory</li>

  <!-- BEGIN /Users/aroth/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/hotwire_combobox-0.1.40/app/views/hotwire_combobox/_pagination.html.erb -->
<li id="mission_supervisor_id__hw_combobox_pagination__wrapper" data-hw-combobox-target="endOfOptionsStream" data-input-type="undefined" aria-hidden="true">
  <turbo-frame loading="lazy" id="mission_supervisor_id__hw_combobox_pagination"></turbo-frame>
</li><!-- END /Users/aroth/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/hotwire_combobox-0.1.40/app/views/hotwire_combobox/_pagination.html.erb -->
</ul><!-- END /Users/aroth/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/hotwire_combobox-0.1.40/app/views/hotwire_combobox/combobox/_paginated_listbox.html.erb -->
  <!-- BEGIN /Users/aroth/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/hotwire_combobox-0.1.40/app/views/hotwire_combobox/combobox/_dialog.html.erb --><div tabindex="-1" data-hw-combobox-target="dialogFocusTrap"></div>

<div class="hw-combobox__dialog__wrapper">
  <dialog class="hw-combobox__dialog" role="dialog" data-action="keydown->hw-combobox#navigate" data-hw-combobox-target="dialog" style="--hw-visual-viewport-height: 525px;">
    <label class="hw-combobox__dialog__label" for="mission_supervisor_id-hw-dialog-combobox"></label>
    <input id="mission_supervisor_id-hw-dialog-combobox" role="combobox" class="hw-combobox__dialog__input" autofocus="autofocus" type="text" data-action="input->hw-combobox#filter keydown->hw-combobox#navigate click@window->hw-combobox#closeOnClickOutside" data-hw-combobox-target="dialogCombobox" aria-controls="mission_supervisor_id-hw-dialog-listbox" aria-owns="mission_supervisor_id-hw-dialog-listbox" aria-autocomplete="both" aria-activedescendant="">
    <ul id="mission_supervisor_id-hw-dialog-listbox" class="hw-combobox__dialog__listbox" role="listbox" data-hw-combobox-target="dialogListbox"></ul>
</dialog></div><!-- END /Users/aroth/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/hotwire_combobox-0.1.40/app/views/hotwire_combobox/combobox/_dialog.html.erb -->
</fieldset>
josefarias commented 6 months ago

Fixed! Merging soon. Thanks for an excellent report and diagnosis @aroth, I've added you as co-author. You were spot-on with the problem.

Kinda hate to pass that boolean to the method. But that's a refactoring for another day.

aroth commented 6 months ago

Excellent, thank you very much @josefarias!