-
Notifications
You must be signed in to change notification settings - Fork 420
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
contextPad: allow keyboard trigger #719
Conversation
bddc510
to
0004960
Compare
c11455c
to
7d95852
Compare
@@ -294,6 +294,9 @@ Overlays.prototype.remove = function(filter) { | |||
|
|||
}; | |||
|
|||
Overlays.prototype.isShown = function() { |
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.
Please add a test case that verifies the existence of this API.
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.
You're right, I added it now
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.
What I was asking for is covering the Overlays#isShown
🙈 (OverlaysSpec
or something). Will add the test case now. 👍
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.
ah sorry, I misread that
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.
Added via 07e28f0.
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.
Looks good, thanks!
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.
Added another test to verify the newly added ContextPad
API, which I renamed to ContextPad#triggerEntry
(for clarity).
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.
We did not return a value anymore from trigger
. Fixed + aadded test case to it.
7d95852
to
d44da08
Compare
01674c6
to
e4e5d7d
Compare
This allows us to understand if overlays are currently shown (or hidden).
Allows manual triggering of entries.
e4e5d7d
to
6d072d0
Compare
Related to bpmn-io/bpmn-js#1785, see bpmn-io/bpmn-js#1785 (comment)