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

[CLOSED] Cmd-Z/Y no longer works in inline color editor #4596

Open
core-ai-bot opened this issue Aug 29, 2021 · 11 comments
Open

[CLOSED] Cmd-Z/Y no longer works in inline color editor #4596

core-ai-bot opened this issue Aug 29, 2021 · 11 comments

Comments

@core-ai-bot
Copy link
Member

Issue by njx
Wednesday Aug 28, 2013 at 23:30 GMT
Originally opened as adobe/brackets#4989


  1. Open an inline color editor on a color
  2. Change the color in the editor
  3. Cmd-Z

Result: Doesn't undo. I'm almost 100% positive this used to work. (The unit tests all pass, but they rely on synthesizing keyboard events.)

I haven't tried this on Win to see if it's failing there too.

Workaround: Click the color swatch for the original color, in the upper-right of the colorpicker, to revert to the original color.

@core-ai-bot
Copy link
Member Author

Comment by njx
Wednesday Aug 28, 2013 at 23:42 GMT


Might be similar to the problem #4315 was trying to fix.

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Wednesday Aug 28, 2013 at 23:43 GMT


Works in Windows Sprint 30 RC1

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Wednesday Aug 28, 2013 at 23:43 GMT


I take it back, it doesn't work in Windows Sprint 30 RC1

@core-ai-bot
Copy link
Member Author

Comment by njx
Wednesday Aug 28, 2013 at 23:46 GMT


Yes, this is the same general issue as the problem in #4315, #418, etc. (thanks to@iwehrman for pointing this out): we can't get a keyboard event in JS for a shortcut that is registered to a menu item, because the shell short-circuits all such key events to call the Brackets command directly. The alternative pull requests #4341 and adobe/brackets-shell#268 (which replaced #4315) won't address this, I think, because they're only about (JS) modal dialogs. I haven't thought through the issue enough to know whether that fix could be generalized to deal with this as well.

@core-ai-bot
Copy link
Member Author

Comment by njx
Wednesday Aug 28, 2013 at 23:47 GMT


Assigning to@RaymondLim since he's thought about this problem for modal dialogs--could that fix be applied to cases like the inline color editor as well?

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Thursday Aug 29, 2013 at 02:39 GMT


Nominating for sprint 31.

@core-ai-bot
Copy link
Member Author

Comment by njx
Thursday Aug 29, 2013 at 16:46 GMT


@RaymondLim - we ended up deciding to move this to a backlog story about improving our key event handling in general: https://trello.com/c/I07x1pO0. Closing as move to backlog.

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Friday Aug 30, 2013 at 21:23 GMT


Reopen it since the issue is caused by our call of EditorManager.getFocusedEditor(). When the user clicks anywhere inside the inline color editor, calling EditorManager.getFocusedEditor will return null and therefore undo/redo is failing.

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Tuesday Sep 03, 2013 at 16:39 GMT


@njx I don't think we should be pre-empting the Cmd-Z/Y in menus like we did for modal dialogs since we also need to make the Undo/Redo menu item clicks work for both native and in-browser menus.

So instead of calling EditorManager.getFocusedEditor() in handleUndoRedo(), which is failing for non-text inline editor, we need to call EditorManager.getActiveEditor() which can properly undo color change in main editor regardless of whether it is from a menu item click or a keyboard shortcut.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Tuesday Sep 03, 2013 at 22:51 GMT


See Raymond's PR #5033...

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday Sep 10, 2013 at 18:09 GMT


We decided to re-close this bug and just deal with it when we get to the key event handling user story (https://trello.com/c/I07x1pO0).

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

No branches or pull requests

1 participant