Make the modal height and width determined by the content (#9973)

* Try new modal styling. * Fix doubel padding declaration. * Add min width and fix button size. * Set 56px above and below the modal. * Add tabindex=-1 to keep tabbing constrained after click on the content. * Tidy up rebase of PR * chore: Fix up remaining issues * chore: Oops, remove the set height * Fix overflow * Full-width the header * Fix test after changing tabbing
WordPress · Oct 10, 2018 · 286317c · 286317c
1 parent 6ff8d3e
commit 286317c
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 57 deletions.
diff --git a/assets/stylesheets/_variables.scss b/assets/stylesheets/_variables.scss
@@ -29,6 +29,7 @@ $admin-sidebar-width: 160px;
 $admin-sidebar-width-big: 190px;
 $admin-sidebar-width-collapsed: 36px;
 $empty-paragraph-height: $text-editor-font-size * 4;
+$modal-min-width: 360px;
 
 // Visuals
 $shadow-popover: 0 3px 30px rgba($dark-gray-900, 0.1);

diff --git a/assets/stylesheets/_z-index.scss b/assets/stylesheets/_z-index.scss
@@ -16,6 +16,7 @@ $z-layers: (
 	".editor-inserter__tabs": 1,
 	".editor-inserter__tab.is-active": 1,
 	".components-panel__header": 1,
+	".components-modal__header": 10,
 	".edit-post-meta-boxes-area.is-loading::before": 1,
 	".edit-post-meta-boxes-area .spinner": 5,
 	".editor-block-contextual-toolbar": 21,

diff --git a/packages/components/src/modal/frame.js b/packages/components/src/modal/frame.js
@@ -121,7 +121,9 @@ class ModalFrame extends Component {
 				role={ role }
 				aria-label={ contentLabel }
 				aria-labelledby={ contentLabel ? null : labelledby }
-				aria-describedby={ describedby }>
+				aria-describedby={ describedby }
+				tabIndex="-1"
+			>
 				{ children }
 			</div>
 		);

diff --git a/packages/components/src/modal/style.scss b/packages/components/src/modal/style.scss
@@ -8,104 +8,93 @@
 	background-color: rgba($white, 0.4);
 	z-index: z-index(".components-modal__screen-overlay");
 
-	// Animate appearance.
+	// This animates the appearance of the white background.
 	@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;
+	// On small screens the content needs to be full width because of limited
+	// space.
+	position: absolute;
 	top: 0;
 	right: 0;
 	bottom: 0;
 	left: 0;
+	box-sizing: border-box;
 	margin: 0;
+	border: $border-width solid $light-gray-500;
+	background: $white;
+	box-shadow: $shadow-modal;
+	overflow: auto;
 
-	// Show slightly bigger on small screens.
+	// Show a centered modal on bigger screens.
 	@include break-small() {
-		position: absolute;
+		top: 50%;
 		right: auto;
 		bottom: auto;
-		max-width: calc(100% - #{ $panel-padding } - #{ $panel-padding });
-		margin-right: -50%;
-		transform: translate(-50%, 0);
-		top: $panel-padding;
 		left: 50%;
-		height: 90%;
+		min-width: $modal-min-width;
+		max-width: calc(100% - #{ $grid-size-large } - #{ $grid-size-large });
+		max-height: calc(100% - #{ $header-height } - #{ $header-height });
+		transform: translate(-50%, -50%);
 
-		// Animate appearance.
+		// Animate the modal frame/contents appearing on the page.
 		@include modal_appear();
 	}
-
-	// Show pretty big on desktop breakpoints.
-	@include break-medium () {
-		max-width: calc(#{ $break-medium } - #{ $panel-padding } - #{ $panel-padding });
-		transform: translate(-50%, -30%);
-		top: 30%;
-		left: 50%;
-		height: 70%;
-	}
 }
 
-// Fix heading to the top.
+// Fix header to the top so it is always there to provide context to the modal
+// if the content needs to be scrolled (for example, on the keyboard shortcuts
+// modal screen).
 .components-modal__header {
 	box-sizing: border-box;
 	border-bottom: $border-width solid $light-gray-500;
-	padding: 0 0 $grid-size 0;
+	padding: 0 $grid-size-large;
 	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});
+	align-items: center;
+	box-sizing: border-box;
 	height: $header-height;
-	padding: $grid-size 0;
+	position: sticky;
+	top: 0;
+	z-index: z-index(".components-modal__header");
+	margin: 0 -#{ $grid-size-large } $grid-size-large;
 
 	.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;
+	h1 {
+		line-height: 1;
+		margin: 0;
 	}
+}
 
-	.components-modal__header-icon-container {
-		display: inline-block;
+.components-modal__header-heading-container {
+	align-items: center;
+	flex-grow: 1;
+	display: flex;
+	flex-direction: row;
+	justify-content: left;
+}
 
-		svg {
-			max-width: $icon-button-size;
-			max-height: $icon-button-size;
-			padding: 8px;
-		}
-	}
+.components-modal__header-icon-container {
+	display: inline-block;
 
-	h1 {
-		line-height: 1;
-		margin: 0;
+	svg {
+		max-width: $icon-button-size;
+		max-height: $icon-button-size;
+		padding: $grid-size;
 	}
 }
 
 // Modal contents.
 .components-modal__content {
 	box-sizing: border-box;
-	overflow: auto;
 	height: 100%;
-	padding: ($header-height+$panel-padding) $panel-padding $panel-padding $panel-padding;
-
-	&:focus {
-		outline: $border-width solid $dark-gray-500;
-	}
+	padding: 0 $grid-size-large $grid-size-large;
 }
diff --git a/test/e2e/specs/a11y.test.js b/test/e2e/specs/a11y.test.js
@@ -50,7 +50,7 @@ describe( 'a11y', () => {
 
 		// Click to move focus to an element after the last tabbable within the
 		// modal.
-		await page.click( '.components-modal__content' );
+		await page.click( '.components-modal__content *:last-child' );
 
 		await page.keyboard.press( 'Tab' );