Skip to content
This repository was archived by the owner on Oct 4, 2021. It is now read-only.

Fix 29887 take 2#998

Open
iainx wants to merge 2 commits intomono:mainfrom
iainx:fix-29887-2
Open

Fix 29887 take 2#998
iainx wants to merge 2 commits intomono:mainfrom
iainx:fix-29887-2

Conversation

@iainx
Copy link
Copy Markdown
Contributor

@iainx iainx commented Jul 31, 2015

This is the more generic version of #995 where the TextEditor has implemented the IKeyHandler interface, which the command manager then checks to see if the text editor is currently overriding the key press.

Fixes the issues where Escape wouldn't close the Insert Extracted Method overlay, or wouldn't close the autocompletion dialog if it was assigned to a command. (Bug 29887)

It also prevents essential keys from being overridden if they are assigned to a command such as alphanumeric keys or Tab, Return or Backspace.

iain holmes added 2 commits July 31, 2015 19:24
IKeyHandler is an interface that allows the current EditMode to say whether or not a key
combination is reserved in that context, or whether it can be passed through to the command
dispatch
Before passing a key press to the command dispatch, check with the currently focused widget
whether or not it is currently overriding the command key mappings for that particular key.
This is so that, for example, Escape will still work when an autocomplete popover is visible
or the extract method insert point is visible, even if Escape has been assigned to a command
@Therzok
Copy link
Copy Markdown
Contributor

Therzok commented Aug 22, 2015

This lgtm.

@slluis
Copy link
Copy Markdown
Member

slluis commented Aug 24, 2015

It looks ok to me. @mkrueger can you review?

@mkrueger
Copy link
Copy Markdown
Contributor

I think that shadows a problem somewhere else. It used to work AFAIK.
What about other components than the text editor ?

Other than that it'll work that way.
But it looks like we're digging a hole here that gets deeper :/. However I don't know enough about the command manager problem and how it can be resolved. Merge if really nothing other will help.

Base automatically changed from master to main March 9, 2021 14:17
@directhex directhex requested a review from slluis as a code owner March 9, 2021 14:17
@akoeplinger akoeplinger changed the base branch from main to master March 15, 2021 17:02
Base automatically changed from master to main March 15, 2021 17:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants