Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Fixed an issue with not removing the resize handler when user clicks RMB #10416
Fixed an issue with not removing the resize handler when user clicks RMB #10416
Changes from all commits
40a0d5e
3c62c52
ef41105
3186034
b7ca869
edf886b
a8539ed
0471a26
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that it is ok that the handler does not disappear after triggering the context menu. The origin bug is still fixed, the handler does not follow the mouse after unclicking the context menu:
By removing these lines, we do not add new code to maintain, and the fix still seems to be working. So why not do it 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you can see, there UI element left behind. Title of the issue says that: "The resize handler should be removed when user clicks RMB".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better code health, I'd remove the lines. UI, in this case, still works fine for me. What do you think @AMBudnik @krzysztofspilka?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify. Current PR fixes the issue by hiding/removing the handler right after the RMB is clicked. I believe it's a bad UI/UX design that some elements just disappear after mouse-clicking.
My proposition is not to hide/remove the handler and make it just inactive for RMB.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine with me 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@budnix @wszymanski @AMBudnik
Thanks for discussing this! I think it all comes down to two options that we have:
google-beh.mp4
or
default
in this case.excel-beh.mp4
Solution no. 2 blocks the UI outside (behind) the context menu, which is exactly what the OS-context menu does. It's also more consistent with how Handsontable interacts with other plugins. For example, in v12.4.0 a mouse cursor over the dropdown menu button doesn't give you the
on-hover
visual feedback. The cursor style remainsdefault
.That being said, I'm fond of solution no. 2 (hide the manual-resize indicator).
See the screen recording below for reference.
Screen.Recording.2023-06-17.at.11.58.06.mov