feat(modal): submit and hide modal on enter #3542
Conversation
| // If Enter pressed, submit modal | ||
| if (evt.keyCode === KeyCodes.ENTER && this.isVisible && !this.noSubmitOnEnter) { | ||
| this.onOk() | ||
| } |
There was a problem hiding this comment.
This would close the modal even in there are cancel buttons, etc. It assumes the user wants the default action to be OK (and not Cancel)
There was a problem hiding this comment.
Would it ok to pass the default action via props?
There was a problem hiding this comment.
It would probably be better to have some option(s) to specify which default rendered buttons have focus when the modal opens (defaulting to no buttons having focus, as autofocusing an input or button on a modal will usually only announce the button/input but not the modal content when using screen readers.
A more robust approach is to have users listen for the shown event, and then focus the element they want (be it a button, input, or other focusable content control).
The shown event includes a reference to the modal element in the bvEvent object (the target property). Once shown, you can then use bvEvent.target.querySelector('some selector criteria)` to find an element and focus it.
Codecov Report
@@ Coverage Diff @@
## dev #3542 +/- ##
==========================================
- Coverage 99.25% 99.23% -0.03%
==========================================
Files 224 224
Lines 4315 4317 +2
Branches 1231 1232 +1
==========================================
+ Hits 4283 4284 +1
- Misses 26 27 +1
Partials 6 6
Continue to review full report at Codecov.
|
|
It probably would be better to handle this in the app logic rather than inside the modal component. i.e. listening for This PR assumes that |
| noSubmitOnEnter: { | ||
| type: Boolean, | ||
| default: false | ||
| }, |
There was a problem hiding this comment.
This also assumes people want to close modal on ENTER, which is currently not the default behavior and would be considered a breaking change.
There was a problem hiding this comment.
Also, submit assumes a form is being used... so its probably not the best name for the prop, as modals don't "submit"
|
msgBoxConfirm and msgBoxOk doesn’t provide access to buttons via $ref |
|
If you pass an ID to the msgBox methods, you can then listen for |
|
If you had a msgBox with an OK and CANCEL button, and the user had CANCEL focused (i.e. they tabbed to CANCEL), your enter handler would trigger the OK action, not the cancel action (which is what the user would be expecting) |
|
What would be better is a prop called It would need tests for coverage, and documentation on the prop, with a note that it only works with the default built in footer buttons. and a note that auto-focusing a control inside the modal may prevent the modal content from being read out for screen reader users. |
|
@tmorehouse thank you! |
|
When will we have this feature? |
Describe the PR
Browser
alertandconfirmmodal windows are submitted on Enter.Bootstrap-vuedoesn't support such behaviour. This PR fixes this.PR checklist
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
The PR fulfills these requirements:
devbranch, not themasterbranch[...] (fixes #xxx[,#xxx]), where "xxx" is the issue number)fix(alert): not alerting during SSR render,docs(badge): update pill examples, fix typos,chore: fix typo in README, etc). This is very important, as theCHANGELOGis generated from these messages.If new features/enhancement/fixes are added or changed:
package.jsonfor slot and event changes)If adding a new feature, or changing the functionality of an existing feature, the PR's
description above includes: