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

contextPad: allow keyboard trigger #719

Merged
merged 4 commits into from
Dec 12, 2022
Merged

Conversation

smbea
Copy link
Contributor

@smbea smbea commented Dec 7, 2022

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Dec 7, 2022
@smbea smbea marked this pull request as draft December 7, 2022 19:50
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Dec 7, 2022
@smbea smbea force-pushed the contextpad-allow-keyboard-trigger branch 3 times, most recently from bddc510 to 0004960 Compare December 7, 2022 20:05
@smbea smbea marked this pull request as ready for review December 7, 2022 20:13
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Dec 7, 2022
@smbea smbea requested review from nikku, a team and marstamm and removed request for a team December 7, 2022 20:15
@smbea smbea changed the title ContextPad: allow keyboard trigger contextPad: allow keyboard trigger Dec 7, 2022
@smbea smbea force-pushed the contextpad-allow-keyboard-trigger branch 3 times, most recently from c11455c to 7d95852 Compare December 12, 2022 11:05
@smbea smbea requested a review from nikku December 12, 2022 11:11
@@ -294,6 +294,9 @@ Overlays.prototype.remove = function(filter) {

};

Overlays.prototype.isShown = function() {
Copy link
Member

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.

Copy link
Contributor Author

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

Copy link
Member

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. 👍

Copy link
Contributor Author

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added via 07e28f0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

Copy link
Member

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).

Copy link
Member

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.

@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Dec 12, 2022
@smbea smbea force-pushed the contextpad-allow-keyboard-trigger branch from 7d95852 to d44da08 Compare December 12, 2022 12:02
@smbea smbea requested a review from nikku December 12, 2022 12:06
@smbea smbea added the needs review Review pending label Dec 12, 2022 — with bpmn-io-tasks
@smbea smbea removed the in progress Currently worked on label Dec 12, 2022
@nikku nikku force-pushed the contextpad-allow-keyboard-trigger branch from 01674c6 to e4e5d7d Compare December 12, 2022 14:24
smbea and others added 4 commits December 12, 2022 15:30
@nikku nikku force-pushed the contextpad-allow-keyboard-trigger branch from e4e5d7d to 6d072d0 Compare December 12, 2022 14:30
@smbea smbea merged commit e5bafd2 into develop Dec 12, 2022
@smbea smbea deleted the contextpad-allow-keyboard-trigger branch December 12, 2022 14:36
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Dec 12, 2022
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

Successfully merging this pull request may close these issues.

2 participants