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

feat: add hook useOnClickOutside #1006

Merged
merged 9 commits into from
Sep 19, 2023
Merged

Conversation

NikitaCG
Copy link
Contributor

No description provided.

@gravity-ui-bot
Copy link
Contributor

Preview is ready.

@NikitaCG NikitaCG changed the title feat/add use on click outside feat: add hook useOnClickOutside Sep 13, 2023
@NikitaCG NikitaCG force-pushed the feat/add-use-on-click-outside branch from 9b83c41 to 9d53847 Compare September 14, 2023 08:36
};

window.addEventListener('click', callback, {capture: true});
window.addEventListener('touchstart', callback, {capture: true});
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be touchend event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends on how we decide. I assumed that we need a callback to be triggered as soon as the user touches another place. If you set touchend, it will work only when the user releases his finger, it may take some time before that

Copy link
Contributor

Choose a reason for hiding this comment

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

But we use click event not mousedown (click happens after mouseup), there should be some consistency

Copy link
Contributor

Choose a reason for hiding this comment

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

I did some research, most popular UI libraries either use click/touchend event or let specify event. For example, MUI, ChakraUI

@NikitaCG NikitaCG force-pushed the feat/add-use-on-click-outside branch 2 times, most recently from 0442647 to 4380d3c Compare September 15, 2023 11:20
@NikitaCG NikitaCG force-pushed the feat/add-use-on-click-outside branch from 4380d3c to 2e9b118 Compare September 15, 2023 23:17
};

window.addEventListener('mousedown', callback, {capture: true});
window.addEventListener('touchend', callback, {capture: true});
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be consistent here, mousedown and touchstart or mouseup and touchend

@NikitaCG NikitaCG force-pushed the feat/add-use-on-click-outside branch from 3d2f4fa to ec50e1a Compare September 19, 2023 12:37
@NikitaCG NikitaCG merged commit 6e83459 into main Sep 19, 2023
3 checks passed
@NikitaCG NikitaCG deleted the feat/add-use-on-click-outside branch September 19, 2023 15:18
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.

3 participants