Skip to content

feat(modal): submit and hide modal on enter #3542

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.

Sign up for GitHub

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
wants to merge 1 commit into from
Closed

feat(modal): submit and hide modal on enter #3542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/components/modal/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export interface BvMsgBoxOptions {
noFade?: boolean
noCloseOnBackdrop?: boolean
noCloseOnEsc?: boolean
noSubmitOnEnter?: boolean
headerBgVariant?: string
headerBorderVariant?: string
headerTextVariant?: string
Expand Down
12 changes: 10 additions & 2 deletions src/components/modal/modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ export const props = {
type: Boolean,
default: false
},
noSubmitOnEnter: {
type: Boolean,
default: false
},
Copy link
Member

Choose a reason for hiding this comment

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

This also assumes people want to close modal on ENTER, which is currently not the default behavior and would be considered a breaking change.

Copy link
Member

Choose a reason for hiding this comment

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

Also, submit assumes a form is being used... so its probably not the best name for the prop, as modals don't "submit"

noEnforceFocus: {
type: Boolean,
default: false
Expand Down Expand Up @@ -600,11 +604,15 @@ export const BModal = /*#__PURE__*/ Vue.extend({
onClose() {
this.hide('headerclose')
},
onEsc(evt) {
onKeyDown(evt) {
// If ESC pressed, hide modal
if (evt.keyCode === KeyCodes.ESC && this.isVisible && !this.noCloseOnEsc) {
this.hide('esc')
}
// If Enter pressed, submit modal
if (evt.keyCode === KeyCodes.ENTER && this.isVisible && !this.noSubmitOnEnter) {
this.onOk()
}
Copy link
Member

@tmorehouse tmorehouse Jun 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

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)

Copy link
Author

Choose a reason for hiding this comment

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

Would it ok to pass the default action via props?

Copy link
Member

Choose a reason for hiding this comment

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

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.

},
// Document focusin listener
focusHandler(evt) {
Expand Down Expand Up @@ -850,7 +858,7 @@ export const BModal = /*#__PURE__*/ Vue.extend({
'aria-hidden': this.isVisible ? null : 'true',
'aria-modal': this.isVisible ? 'true' : null
},
on: { keydown: this.onEsc, click: this.onClickOut }
on: { keydown: this.onKeyDown, click: this.onClickOut }
},
[modalDialog]
)
Expand Down