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.

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

Jump to bottom

feat(NcModal): Allow to configure if the modal is closed when clicked outside #4454

Merged
merged 1 commit into from Aug 24, 2023
Merged

feat(NcModal): Allow to configure if the modal is closed when clicked outside #4454

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 24 additions & 1 deletion src/components/NcModal/NcModal.vue
View file
Open in desktop
Expand Up @@ -252,7 +252,7 @@ export default {
spreadNavigation ? 'modal-wrapper--spread-navigation' : ''
]"
class="modal-wrapper"
@mousedown.self="close">
@mousedown.self="handleClickModalWrapper">
<!-- Navigation button -->
<transition name="fade-visibility" appear>
<NcButton v-show="hasPrevious"
Expand Down Expand Up @@ -420,13 +420,24 @@ export default {
return ['small', 'normal', 'large', 'full'].includes(size)
},
},

/**
* Declare if the modal can be closed
*/
canClose: {
type: Boolean,
default: true,
},

/**
* Close the modal if the user clicked outside of the modal
* Only relevant if `canClose` is set to true.
*/
closeOnClickOutside: {
type: Boolean,
default: true,
},
Comment on lines +432 to +439
Copy link
Contributor

Choose a reason for hiding this comment

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

In a standard form of passing props Vue enforces HTML standard aka:

<input /> <!-- enabled-->
<input disabled /> <!-- disabled -->

But boolean props with default true requires to always use longhand form over HTML Standard:

<NcModal /> <!-- closes on click -->
<NcModal close-on-click-outside /> <!-- also closes on click, passing bool prop does nothing -->
<NcModal :close-on-click-outside="false" /> <!-- changes behavior, doesn't close on click -->
<NcModal close-on-click-outside="false" /> <!-- casts string "false" to true and closes on click -->

because of that it is usually recommended to make bool props with false default value:

<!-- With default false boolean prop -->
<NcModal /> <!-- closes on click -->
<NcModal ignore-click-outside /> <!-- changes behavior, doesn't close on click -->

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you :)
I know but did not wanted to make this a breaking change by changing the default behavior and a negated property seems to be confusing.
Maybe we can risk another breaking change?


/** Makes the modal backdrop black if true */
dark: {
type: Boolean,
Expand Down Expand Up @@ -616,6 +627,18 @@ export default {
}
},

/**
* Handle click on modal wrapper
* If `closeOnClickOutside` is set the modal will be closed
*
* @param {MouseEvent} event The click event
*/
handleClickModalWrapper(event) {
if (this.closeOnClickOutside) {
this.close(event)
}
},

/**
* @param {KeyboardEvent} event - keyboard event
*/
Expand Down
59 changes: 59 additions & 0 deletions tests/unit/components/NcModal/modal.spec.js
View file
Open in desktop
@@ -0,0 +1,59 @@
/**
* @copyright Copyright (c) 2023 Ferdinand Thiessen <opensource@fthiessen.de>
*
* @author Ferdinand Thiessen <opensource@fthiessen.de>
*
* @license AGPL-3.0-or-later
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

import { mount } from '@vue/test-utils'
import NcModal from '../../../../src/components/NcModal/NcModal.vue'

describe('NcModal', () => {
it('closes on click outside with `canClose`', async () => {
const wrapper = mount(NcModal, { propsData: { canClose: true, title: 'modal' } })
expect(wrapper.html().includes('modal-wrapper')).toBe(true)

expect(wrapper.emitted('update:show')).toBe(undefined)

await wrapper.find('.modal-wrapper').trigger('mousedown')
// One emit('update:show', false)
expect(wrapper.emitted('update:show')).toEqual([[false]])
})

it('not closes on click outside when `canClose` is false', async () => {
const wrapper = mount(NcModal, { propsData: { canClose: false, title: 'modal' } })
expect(wrapper.html().includes('modal-wrapper')).toBe(true)

expect(wrapper.emitted('update:show')).toBe(undefined)

await wrapper.find('.modal-wrapper').trigger('mousedown')
// One emit('update:show', false)
expect(wrapper.emitted('update:show')).toEqual(undefined)
})

it('not closes on click outside when `canClose` is true but `closeOnClickOutside` is false', async () => {
const wrapper = mount(NcModal, { propsData: { canClose: true, closeOnClickOutside: false, title: 'modal' } })
expect(wrapper.html().includes('modal-wrapper')).toBe(true)

expect(wrapper.emitted('update:show')).toBe(undefined)

await wrapper.find('.modal-wrapper').trigger('mousedown')
// One emit('update:show', false)
expect(wrapper.emitted('update:show')).toEqual(undefined)
})
})