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] Handle keyboard events for nested dialogs properly #3308

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

[CLOSED] Handle keyboard events for nested dialogs properly #3308

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

Comments

@core-ai-bot
Copy link
Member

Issue by njx
Sunday Apr 21, 2013 at 00:49 GMT
Originally opened as adobe/brackets#3522


Instead of attaching a keydown listener for each dialog, keep track of the stack of nested dialogs and have the listener always deal with the topmost dialog. Fixes #3505.

(Ideally we wouldn't have nested dialogs at all, but the current Extension Manager implementation requires it. In the future, we might get rid of one level of dialogs there.)

Note that when #3086 (or similar) is merged, we can clean this up a bit by just having the stack hold Dialog objects and storing autoDismiss in the object.

As an aside, we should add some unit tests for the dialog infrastructure. Maybe the right time to do that is when we review #3086.


njx included the following code: https://github.com/adobe/brackets/pull/3522/commits

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Tuesday Apr 23, 2013 at 20:32 GMT


@njx should we not bother merging this until a later sprint, given the change in Extension Manager plans?

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday Apr 23, 2013 at 20:54 GMT


Yup, no need to review this now.

@core-ai-bot
Copy link
Member Author

Comment by iwehrman
Friday May 03, 2013 at 20:25 GMT


For comparison, here's my (very quick, possibly buggy, probably incomplete) solution: b79284afe8c615cde69e6b441d57882ea2ba641b

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday May 10, 2013 at 19:30 GMT


@njx do you have a preference between this and@iwehrman's implementation? Ian's is definitely a simpler diff. We'd need to port over your Esc handling fix, and probably do some minor cleanups (e.g. add JSDocs and leading "_" to keydownListeners), but that might be enough to do the trick.

@core-ai-bot
Copy link
Member Author

Comment by njx
Friday May 10, 2013 at 19:41 GMT


Yeah, I like Ian's better, though I haven't looked to see if it's equivalent.@iwehrman, mind submitting yours to the brackets repo?

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday May 14, 2013 at 21:39 GMT


Closing in favor of #3785.

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