-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix: caret position when editing block comments #9153
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: caret position when editing block comments #9153
Conversation
| e.stopPropagation(); | ||
| }); | ||
| // Don't let the pointerdown event get to the workspace. | ||
| browserEvents.conditionalBind(textArea, 'pointerdown', this, (e: Event) => { |
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.
Instead of adding it here, what do you think of modifying the bind call in bubble to bind to the focusable area if it exists and the background otherwise? I'm not sure what the effects will be either way, but it seemed like that might have been the intent of the existing bind.
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.
That doesn't work because the event listener on the background needs to be different. That event listener starts the gesture which might be responsible for dragging the bubble. If you stop the propagation there, dragging doesn't work. And if you moved that event listener to the textarea instead, then you wouldn't be able to drag the bubble by grabbing the background, which is how you currently move the bubble. Dragging on the textarea doesn't move the bubble, it highlights the text like you would expect for textareas. So I think what I have is more correct.
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.
Ah, makes sense! Thanks for helping me understand what each piece is doing. =)
Aligns with RaspberryPiFoundation/blockly#9153 Workspace comments need a separate fix from Blockly.
Aligns with RaspberryPiFoundation/blockly#9153 Workspace comments need a separate fix from Blockly.
* Temporary fix for preview branch to fix flyout navigation * Improve navigation behaviour between toolbox and flyout Re-focus previously focused block in the flyout if possible. * Add context menu CSS, drop old workaround The workaround equivalent shipped in 12.1.0. * Fix move keyboard shortcut name The format was aligned with the others in the plugin. * Handle Blockly focus for blocks with plus / minus buttons These buttons are removed from the DOM and re-rendered when interacted with. As a result focus is lost. This change re-focuses the block when this happens. * Don't manage focus for +/- buttons for initial render * Ignore the previous flyout node if the flyout was closed * Ignore drag and selected events in shouldEventHideFlyout * Also ignore BLOCK_MOVE event for shouldEventHideFlyout * Focus block after interaction with + / - buttons This includes the expand / collapse buttons for functions. * Somewhat crufty icon focus fixes. Hard to tell if the opacity is intended, it's quite odd really but would be a meaningful visual change. * Fix moving caret in block comments Aligns with RaspberryPiFoundation/blockly#9153 Workspace comments need a separate fix from Blockly. * Revert unintended change * Add translations used by the keyboard navigation plugin. * Update to Blockly beta releases * Clear touch identifier as per Blockly fix See RaspberryPiFoundation/blockly#9172 --------- Co-authored-by: Robert Knight <[email protected]>
The basics
The details
Resolves
Fixes #9125
Proposed Changes
Adds a pointerdown event that just captures the event and stops it from propagating.
Reason for Changes
There was a pointerdown event that was being fired on the workspace when clicking on the block comment's textarea. I didn't look further into figuring out exactly why that caused the caret not to move.
This approach of capturing the event is already being used in the workspace comment textarea, and those don't have this problem (they have other problems, but this isn't one of them).
Test Coverage
Manual testing including with bke.
Documentation
Additional Information