From 86dad5f40a9892fa70b9674f7f25cf2ca44733ea Mon Sep 17 00:00:00 2001 From: Joen Asmussen Date: Fri, 5 Oct 2018 20:36:20 -0400 Subject: [PATCH] Don't focus close button on modal open/Try a modal appear animation (#9900) * Add a modal appear animation * Explore a way to not show tooltip --- edit-post/assets/stylesheets/_animations.scss | 14 ++++ edit-post/assets/stylesheets/main.scss | 9 --- .../keyboard-shortcut-help-modal/index.js | 2 - packages/components/src/modal/index.js | 36 ++++----- packages/components/src/modal/style.scss | 78 ++++++++++++------- test/e2e/specs/a11y.test.js | 12 +-- 6 files changed, 87 insertions(+), 64 deletions(-) diff --git a/edit-post/assets/stylesheets/_animations.scss b/edit-post/assets/stylesheets/_animations.scss index 75d3c49e093dd..7b14a7036e6d2 100644 --- a/edit-post/assets/stylesheets/_animations.scss +++ b/edit-post/assets/stylesheets/_animations.scss @@ -62,3 +62,17 @@ @mixin animate_rotation($speed: 1s) { animation: rotation $speed infinite linear; } + +@keyframes modal-appear { + from { + margin-top: $grid-size * 4; + } + to { + margin-top: 0; + } +} + +@mixin modal_appear() { + animation: modal-appear 0.1s ease-out; + animation-fill-mode: forwards; +} diff --git a/edit-post/assets/stylesheets/main.scss b/edit-post/assets/stylesheets/main.scss index dc6a1dfb6c36f..4222907c5c1cf 100644 --- a/edit-post/assets/stylesheets/main.scss +++ b/edit-post/assets/stylesheets/main.scss @@ -278,12 +278,3 @@ body.gutenberg-editor-page { .ephox-snooker-resizer-bar-dragging { background: $blue-medium-500; } - -// This style is defined here instead of the modal component -// because the modal should be context agnostic. -.components-modal__content { - height: calc(100% - #{ $header-height } - #{ $admin-bar-height }); - body.is-fullscreen-mode & { - height: calc(100% - #{ $header-height }); - } -} diff --git a/edit-post/components/keyboard-shortcut-help-modal/index.js b/edit-post/components/keyboard-shortcut-help-modal/index.js index cd9715f3123bf..7dbf0aa310fbd 100644 --- a/edit-post/components/keyboard-shortcut-help-modal/index.js +++ b/edit-post/components/keyboard-shortcut-help-modal/index.js @@ -91,11 +91,9 @@ export function KeyboardShortcutHelpModal( { isModalActive, toggleModal } ) { closeLabel={ __( 'Close' ) } onRequestClose={ toggleModal } > - { shortcutConfig.map( ( config, index ) => ( ) ) } - ) } diff --git a/packages/components/src/modal/index.js b/packages/components/src/modal/index.js index 5be41267ac645..2316331d0ecc6 100644 --- a/packages/components/src/modal/index.js +++ b/packages/components/src/modal/index.js @@ -104,10 +104,10 @@ class Modal extends Component { /** * Stop all onMouseDown events propagating further - they should only go to the modal - * @param {string} ev Event object + * @param {string} event Event object */ - stopEventPropagationOutsideModal( ev ) { - ev.stopPropagation(); + stopEventPropagationOutsideModal( event ) { + event.stopPropagation(); } /** @@ -130,12 +130,10 @@ class Modal extends Component { ...otherProps } = this.props; - const headingId = ( - aria.labelledby || - 'components-modal-header-' + instanceId - ); + const headingId = aria.labelledby || `components-modal-header-${ instanceId }`; - // Disable reason: this stops mouse events from triggering tooltips and other elements underneath the modal overlay + // Disable reason: this stops mouse events from triggering tooltips and + // other elements underneath the modal overlay. /* eslint-disable jsx-a11y/no-static-element-interactions */ return createPortal(
- -
+ { ...otherProps } + > +
+ { children }
diff --git a/packages/components/src/modal/style.scss b/packages/components/src/modal/style.scss index 426d9aef972ad..e529a072fe84c 100644 --- a/packages/components/src/modal/style.scss +++ b/packages/components/src/modal/style.scss @@ -7,10 +7,18 @@ left: 0; background-color: rgba($white, 0.4); z-index: z-index(".components-modal__screen-overlay"); + + // Animate appearance. + @include fade-in(); } // The modal window element. .components-modal__frame { + border: $border-width solid $light-gray-500; + background-color: $white; + box-shadow: $shadow-modal; + outline: none; + // In small screens the content needs to be full width. position: fixed; top: 0; @@ -30,6 +38,9 @@ top: $panel-padding; left: 50%; height: 90%; + + // Animate appearance. + @include modal_appear(); } // Show pretty big on desktop breakpoints. @@ -40,50 +51,61 @@ left: 50%; height: 70%; } - - border: $border-width solid $light-gray-500; - background-color: $white; - box-shadow: $shadow-modal; - outline: none; } +// Fix heading to the top. .components-modal__header { box-sizing: border-box; - height: $header-height; border-bottom: $border-width solid $light-gray-500; - padding: $item-spacing $item-spacing $item-spacing $panel-padding; + padding: 0 0 $grid-size 0; display: flex; flex-direction: row; align-items: stretch; justify-content: space-between; -} + position: fixed; + top: 0; + background: $white; + width: calc(100% - #{$panel-padding + $panel-padding}); + height: $header-height; + padding: $item-spacing 0; -.components-modal__header-heading-container { - align-items: center; - flex-grow: 1; - display: flex; - flex-direction: row; - justify-content: left; -} + .components-modal__header-heading { + font-size: 1em; + font-weight: normal; + } -.components-modal__header-heading { - font-size: 1em; - font-weight: normal; -} + .components-modal__header-heading-container { + align-items: center; + flex-grow: 1; + display: flex; + flex-direction: row; + justify-content: left; + } -.components-modal__header-icon-container { - display: inline-block; + .components-modal__header-icon-container { + display: inline-block; - svg { - max-width: $icon-button-size; - max-height: $icon-button-size; - padding: 8px; + svg { + max-width: $icon-button-size; + max-height: $icon-button-size; + padding: 8px; + } + } + + h1 { + line-height: 1; + margin: 0; } } +// Modal contents. .components-modal__content { - // The height of the content is the height of it's parent, minus the header. after that, the offset was 3px. - height: 100%; + box-sizing: border-box; overflow: auto; - padding: $panel-padding; + height: 100%; + padding: ($header-height+$panel-padding) $panel-padding $panel-padding $panel-padding; + + &:focus { + outline: $border-width solid $dark-gray-500; + } } diff --git a/test/e2e/specs/a11y.test.js b/test/e2e/specs/a11y.test.js index 704ae68bba49e..4534a8ac23dac 100644 --- a/test/e2e/specs/a11y.test.js +++ b/test/e2e/specs/a11y.test.js @@ -31,17 +31,17 @@ describe( 'a11y', () => { } ); it( 'constrains focus to a modal when tabbing', async () => { - // Open help modal + // Open keyboard help modal. await pressWithModifier( ACCESS_MODIFIER_KEYS, 'h' ); - // Test that the Close button of the modal is focused when the - // latter is opened. - expect( await isCloseButtonFocused() ).toBe( true ); + // The close button should not be focused by default; this is a strange UX + // experience. + // See: https://github.com/WordPress/gutenberg/issues/9410 + expect( await isCloseButtonFocused() ).toBe( false ); await page.keyboard.press( 'Tab' ); - // Test that the Close button of the modal is focused when the - // latter is opened. + // Ensure the Close button of the modal is focused after tabbing. expect( await isCloseButtonFocused() ).toBe( true ); } );