search

rob-balfre / svelte-select

Svelte Select. A select component for Svelte
https://svelte-select-examples.vercel.app
Other
1.25k stars 175 forks source link

Fix select for touch event on indicator icons #535

Closed miXwui closed 1 year ago

miXwui commented 1 year ago

This PR fixes the touch event on divs/containers that inside the svelte-select class div but outside the select <input />, such as the chevron and prepend icons.

Repro: Click the chevron icon with a touchscreen: https://svelte-select-examples.vercel.app/examples/props/show-chevron

I've tested this on Chrome/Firefox/Edge Android. This can also be easily tested on desktop with Firefox's Responsive Design Mode with Touch Simulation Enabled, as shown here: https://user-images.githubusercontent.com/4538842/215847101-29f38b15-5f80-4029-b4da-9e223ff0f56e.mp4

Touching the icon results in opening the list, and then immediately closing it. On desktop with a mouse, the list opens twice.

Same behavior when clicking the prepend icon: https://svelte-select-examples.vercel.app/examples/slots/prepend

This is because both the mousedown and pointerup events are fired. Adding preventDefault mousedown fixes it.

Addition to: https://github.com/rob-balfre/svelte-select/issues/403 And may fix: https://github.com/rob-balfre/svelte-select/issues/324

P.S: Thank you for creating/maintaining this library! Amazing :raised_hands:

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
svelte-select ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 31, 2023 at 6:26PM (UTC)
rob-balfre commented 1 year ago

@miXwui thanks so much.

rob-balfre commented 1 year ago

@miXwui released in 5.1.4

miXwui commented 1 year ago

Thanks for the quick turnaround!

durkie commented 3 months ago

It looks like this fix is no longer in place. Clicking the chevron on https://svelte-select-examples.vercel.app/examples/props/show-chevron to close the list does not seem to work on desktop Firefox (122.0.1) or desktop Chrome (122.0.6261.129). On mobile safari (ios 17) it doesn't open the list correctly either.

If I drop in to the console and add this fix back: document.getElementsByClassName('svelte-select')[0].addEventListener('mousedown', (e) => { e.preventDefault() }); then it functions normally again.

miXwui commented 3 months ago

I took a quick look and can confirm what @durkie said on Firefox 123.0.1 and Chromium 122.0.6261.111 on desktop. I also tested on mobile.

Dug in a bit and found it was removed in this commit, not sure why — @mBoegvald any insight?

mBoegvald commented 3 months ago

I don't remember the exact behaviour, but by moving mousedown 1 layer deeper my change would allow for no clearing text on blur and keep the old functionality (so i thought), I guess I must've missed clicking the chevron directly, which is also what the test suggests. Sorry!

I can take a look at it, I'm just not sure if I'll have time this week.

mBoegvald commented 1 month ago

Hi everyone,

I apologize for the delay, I forgot to take a look, as I didn't receive any notifications.

I've looked at the bug and realized why the mousedown event was moved. In scenarios where the select element is searchable and you click on the filterText, the text cursor always jumps to the end of the word. This isn't very user-friendly if you want to make changes in the middle of the word.

I'm struggling to make both functionalities work together. If anyone has any ideas, I'm open to suggestions. Otherwise, I might have to live without this feature and move the mousedown event back to restore the chevron functionality.

Sorry for the inconvenience.