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
WordPress · Oct 6, 2018 · 86dad5f · 86dad5f
1 parent 8510c7f
commit 86dad5f
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 64 deletions.
diff --git 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
@@ -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
@@ -91,11 +91,9 @@ export function KeyboardShortcutHelpModal( { isModalActive, toggleModal } ) {
 					closeLabel={ __( 'Close' ) }
 					onRequestClose={ toggleModal }
 				>
-
 					{ shortcutConfig.map( ( config, index ) => (
 						<ShortcutSection key={ index } { ...config } />
 					) ) }
-
 				</Modal>
 			) }
 		</Fragment>

diff --git 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(
 			<div
@@ -152,17 +150,17 @@ class Modal extends Component {
 						labelledby: title ? headingId : null,
 						describedby: aria.describedby,
 					} }
-					{ ...otherProps } >
-					<ModalHeader
-						closeLabel={ closeButtonLabel }
-						isDismissable={ isDismissable }
-						onClose={ onRequestClose }
-						title={ title }
-						headingId={ headingId }
-						icon={ icon }
-					/>
-					<div
-						className={ 'components-modal__content' }>
+					{ ...otherProps }
+				>
+					<div className={ 'components-modal__content' } tabIndex="0">
+						<ModalHeader
+							closeLabel={ closeButtonLabel }
+							headingId={ headingId }
+							icon={ icon }
+							isDismissable={ isDismissable }
+							onClose={ onRequestClose }
+							title={ title }
+						/>
 						{ children }
 					</div>
 				</ModalFrame>

diff --git 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
@@ -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 );
 	} );