-
Notifications
You must be signed in to change notification settings - Fork 72
Keyboard shortcuts #755
base: master
Are you sure you want to change the base?
Keyboard shortcuts #755
Conversation
} | ||
|
||
function composeBoxIsShown() { | ||
return $('#' + composeBoxId).length; |
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.
I'm not very happy with this. Is there any way to get that state of the application other than by accessing the DOM?
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.
@treppo Not really, we just try to minimize the impact of messing with the dom by only changing things rendered inside the specific component. In this case, I would use the key combos to trigger events to specific components, that can deal with the dom manipulation it needs to do the action
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.
@bwagnerr i do trigger events on the specific components. only these events here have to be triggered on document
. and therefore i have to check whether the compose box is open by accessing the dom…
dd08974
to
7a50d7d
Compare
ec1e675
to
eb9ce7e
Compare
Opening this pull–request to track work on #25
so far implemented: