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

AG-13008, AG-13041 Fix Delete/Escape key deleting the annotation in text-edit mode #2605

Merged
merged 12 commits into from
Oct 3, 2024

Conversation

olegat
Copy link
Contributor

@olegat olegat commented Sep 30, 2024

@olegat olegat marked this pull request as ready for review September 30, 2024 16:53
@olegat olegat requested review from alantreadway and a team as code owners September 30, 2024 16:53
Copy link
Member

@lsjroberts lsjroberts left a comment

Choose a reason for hiding this comment

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

There's been a lot of back and forth on deleting annotations due to changes in interactions.

Please make sure any changes continue to work for both of these fixes:

@olegat olegat force-pushed the AG-13008/bugfix_delete_textedit branch from c0eb901 to d572a66 Compare October 1, 2024 13:14
Copy link
Member

@lsjroberts lsjroberts left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Member

@alantreadway alantreadway left a comment

Choose a reason for hiding this comment

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

LGTM

@olegat olegat force-pushed the AG-13008/bugfix_delete_textedit branch from 186dc17 to fceef5c Compare October 2, 2024 08:24
@olegat olegat changed the title AG-13008 Fix Delete/Escape key deleting the annotation in text-edit mode AG-13008, AG-13041 Fix Delete/Escape key deleting the annotation in text-edit mode Oct 2, 2024
@olegat
Copy link
Contributor Author

olegat commented Oct 2, 2024

There's been a lot of back and forth on deleting annotations due to changes in interactions.

Please make sure any changes continue to work for both of these fixes:

I've checked these.

✅ AG-12476 - No regression
✅ AG-12598 - No regression

@olegat olegat force-pushed the AG-13008/bugfix_delete_textedit branch 3 times, most recently from 7b60890 to b0d8041 Compare October 2, 2024 15:11
@olegat
Copy link
Contributor Author

olegat commented Oct 2, 2024

Rebased and resolved conflict by dropping the e2e snapshot update to this file:
packages/ag-charts-website/e2e/zoom.spec.ts-snapshots/zoom-crosshairs-after-wheel-zoom-chromium-linux.png

This fixes a regression caused by the removal of the "mouseBlur" feature in
KeyNavManager.

This is sort of an anti-pattern. Showing the focus after pressing Backspace or
Delete on the keyboard is the "correct" behaviour. However, the correct
behaviour in this scenario feels flimsy.

Rather than restoring the "mouseBlur" feature, we instead create and focus on a
temporary so-called "focus mask" which is added above the canvas. The focus mask
does not respond to pointer events, and puts the canvas-proxy back into focus
whenever any key is pressed. The focus mask is removed once blurred. This mimics
the "mouseBlur" functionality that the use to have.
@olegat olegat force-pushed the AG-13008/bugfix_delete_textedit branch 2 times, most recently from 3db9d5d to 8bf7c81 Compare October 3, 2024 10:06
@olegat olegat merged commit e6d04af into latest Oct 3, 2024
28 of 29 checks passed
@olegat olegat deleted the AG-13008/bugfix_delete_textedit branch October 3, 2024 10: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.

3 participants