Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(slider): fixed the onEnd event to the correct one #1100

Merged
merged 1 commit into from
Mar 29, 2024

Conversation

jkikksa
Copy link
Contributor

@jkikksa jkikksa commented Feb 8, 2024

https://refreshless.com/nouislider/events-callbacks/
image
Судя по описанию событий в noUiSlider корректнее использовать событие change вместо end для обработки события окончания перетаскивания ползунка

Copy link

changeset-bot bot commented Feb 8, 2024

🦋 Changeset detected

Latest commit: 2911003

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@alfalab/core-components-slider Patch
@alfalab/core-components-slider-input Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@core-ds-bot
Copy link
Collaborator

Собрана новая демка.

@reme3d2y
Copy link
Contributor

reme3d2y commented Feb 9, 2024

Привет, а почему это изменение потребовалось? Какой-то баг был?

Исходили из этой логики

image

@jkikksa
Copy link
Contributor Author

jkikksa commented Feb 9, 2024

Привет, а почему это изменение потребовалось? Какой-то баг был?

а тут же видно, что событие end не вызывается когда переместили ползунок стрелками на клавиатуре или тапом в слайдере.

Screen.Recording.2024-02-09.at.12.31.57.mov

@v-gevak
Copy link
Contributor

v-gevak commented Feb 9, 2024

Привет, а почему это изменение потребовалось? Какой-то баг был?

а тут же видно, что событие end не вызывается когда переместили ползунок стрелками на клавиатуре или тапом в слайдере.

Проблема в том, что и start тоже не вызывается. Будет странно, что onEnd вызывается, а onStart нет

@jkikksa
Copy link
Contributor Author

jkikksa commented Feb 9, 2024

Привет, а почему это изменение потребовалось? Какой-то баг был?

а тут же видно, что событие end не вызывается когда переместили ползунок стрелками на клавиатуре или тапом в слайдере.

Проблема в том, что и start тоже не вызывается. Будет странно, что onEnd вызывается, а onStart нет

какие тогда есть предложения? для наших задач нужен доступ к событию change ) но название пропсы onChange уже занято )

@reme3d2y
Copy link
Contributor

Можем попробовать рефку дать для подключения к инстансу noUI

@reme3d2y reme3d2y closed this Feb 12, 2024
@reme3d2y reme3d2y reopened this Feb 12, 2024
@core-ds-bot
Copy link
Collaborator

Собрана новая демка.

@jkikksa
Copy link
Contributor Author

jkikksa commented Feb 12, 2024

но событие change востребовано и судя по описанию вполне может называться onEnd )

It fires when a user stops sliding, when a slider value is changed by 'tap', or on keyboard interaction.

onEnd и onStart в текущей реализации выглядат бесполезными ) они работают только на клики мыши и не реагируют на события клавиатуры
предлагаю удалить onStart чтобы оно не вводило в заблуждение, а на onEnd повесить change

@reme3d2y
Copy link
Contributor

@jkikksa да, давай так сделаем. Только я бы не удалял onStart, а пометил deprecated сначала

'@alfalab/core-components-slider': patch
---

fix(slider): fixed the onEnd event to the correct
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут для ченджлога, надо по-русски написать

@core-ds-bot
Copy link
Collaborator

Собрана новая демка.

@SiebenSieben SiebenSieben merged commit 4b55e0e into master Mar 29, 2024
7 checks passed
@SiebenSieben SiebenSieben deleted the fix/slider-end-event branch March 29, 2024 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants