Skip to content

Conversation

@gonfunko
Copy link
Contributor

The basics

The details

Resolves

Fixes #8842

Proposed Changes

This PR updates Gesture to check for conformance to IContextMenu when dispatching right clicks, rather than hard coding specific kinds of elements.

Although Gesture is still fairly closely tied to specific elements, in general this does open a path for things to just implement IContextMenu to get context menu support. Clicks bubble up from e.g. fields to blocks to the workspace, with each item informing Gesture of a click in turn, and the workspace handler is what ultimately triggers the right click handler. Thus, as long as an element (a) gains focus on (right)click, (b) is a child of the workspace, and (c) implements IContextMenu, Gesture should now trigger display of the context menu when it is right clicked.

While I was here, I also cleaned up the bringing-block-to-front behavior. This was being triggered in various block sub-elements (fields, icons), but because of the bubbling behavior described above, clicks on those will ultimately reach the parent block, so performing bring-to-front when the block becomes the target block covers all these cases, for both left and right clicks.

Copy link
Collaborator

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @gonfunko! Just one comment to double check one of the bringBlockToFront removals that I don't think is covered by tests, but otherwise this LGTM.

Comment on lines -881 to -885
// Note that the order is important here: bringing a block to the front will
// cause it to become focused and showing the field editor will capture
// focus ephemerally. It's important to ensure that focus is properly
// restored back to the block after field editing has completed.
this.bringBlockToFront();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to check, did you validate this bit is no longer needed? I.e. that using keyboard nav to end field editing correctly returns focus to the block? (I think that was the case that necessitated this originally).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAICT behavior is unchanged; if you focus a field via keyboard, hit enter, change it, and hit enter, the field gets focus when editing is committed. If you click on a field, make changes, and hit enter, the parent block gets focus, which is consistent with the behavior I see on the hosted keyboard experiment demo page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update gesture code to check for IContextMenu

3 participants