search

josefarias / hotwire_combobox

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

Dismissing dialog after editing selection in async combobox loses hidden value #180

Closed ILiuz closed 2 weeks ago

ILiuz commented 1 month ago

Bug description

When opening an async single select combobox in dialog mode (on a mobile device), if you first make a selection, then go back and edit the text by using backspace (not removing all text), you will lose the previous selection. If you then close the modal, it will appear as if the old item is still selected, but the value of the hidden element will be blank. Re-opening the dialog again and changing the selection to anything other than the originally selected value, will result in an empty hidden value.

To Reproduce

Steps to reproduce the behavior:

  1. Go to hotwirecombobox.com
  2. Scroll down to 'An async combobox' example
  3. Make sure you have a mobile-sized viewport or are using a mobile device
  4. Make a selection in the combobox
  5. Open the combobox again, and remove some of the text by hitting backspace
  6. Close the dialog by tapping on the sliver of background at the top of the screen
  7. See that the previous selection is present in the combobox
  8. Inspect the HTML to see that the value property of the hidden element is blank

Expected behavior

If you do the same on a large viewport by editing the text with backspace, when you click outside the combobox, it will then re-select the previous item, with a correct hidden element value. I would expect the same on mobile, or at least a match between what is visually selected and what the hidden value is storing. If there is no hidden value, there should be no element selected in the combobox.

If you use the example A basic combobox instead, it will also have a correct hidden element value.

Version Numbers

HotwireCombobox info

Desktop info (if applicable)

Additional context

I'm sorry about the title of this bug report, it was difficult to find the right words without being too verbose.

josefarias commented 3 weeks ago

@ILiuz sorry about the delay here. This is due to a race condition and I really wanted to avoid creating a queue system to avoid those. But I thought about it for a long while and I don't think there's a way around it.

If you get a moment, could I get you to point your gem to this branch and verify that this is fixed there? https://github.com/josefarias/hotwire_combobox/pull/191

Thank you! I really appreciate the well-written report

ILiuz commented 3 weeks ago

@josefarias Thank you so much for taking a look at this!

I couldn't get this working in my actual app which uses esbuild, but on a dummy app with importmaps, everything worked as expected. The hidden input was updated!

Just tangentially related to this issue: For some reason I was not able to get any meaningful code in node_modules when I included your branch in package.json ("@josefarias/hotwire_combobox": "https://github.com/josefarias/hotwire_combobox.git#jose/queue-callbacks") when using esbuild. It was downloaded, but I got mostly just empty folders, so my app failed when trying to build the JS. This is probably just an issue on my part, and I didn't mention that I used esbuild because I was able to reproduce the issue on your docs site, which I assume uses importmaps. I will test on my actual app once you have published on NPM, or if you know what the issue might be, please let me know so I can test more.

josefarias commented 2 weeks ago

@ILiuz thank you for going out of your way to test that. Glad to know it's working. Merged now! Will be a part of the next release.

re: npm — it's set up to ignore everything but the readme and two library builds. But the building only happens on release, and the files are git-ignored. So individual branches don't have the builds and, well, testing WIP via npm is less straight forward. All that to say, this should work once the new version is released. Or you're welcome to create builds from pre-releases using yarn build.