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

Make Placeholder component "element-query-like" responsive. #18745

Merged
merged 3 commits into from
Dec 19, 2019
Merged

Make Placeholder component "element-query-like" responsive. #18745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2468265
Hide Placeholder instructions when smaller than 320px
Nov 20, 2019
e484608
React Resize Aware.
Dec 16, 2019
3ee7908
Fix unit tests
youknowriad Dec 19, 2019
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
10 changes: 8 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions packages/block-editor/src/components/block-pattern-picker/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@

.block-editor-block-pattern-picker__patterns.block-editor-block-pattern-picker__patterns {
display: flex;
justify-content: center;
justify-content: flex-start;
flex-direction: row;
flex-wrap: wrap;
width: 100%;
margin: $grid-size-small 0;
margin: $grid-size-large 0;
padding: 0;
list-style: none;

> li {
list-style: none;
margin: $grid-size;
margin: 0 $grid-size 0 0;
flex-shrink: 1;
max-width: 100px;
}
Expand Down
5 changes: 2 additions & 3 deletions packages/block-editor/src/components/media-placeholder/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,16 +339,15 @@ export class MediaPlaceholder extends Component {
render={ ( { openFileDialog } ) => {
const content = (
<>
<IconButton
<Button
isSecondary
className={ classnames(
'block-editor-media-placeholder__button',
'block-editor-media-placeholder__upload-button'
) }
icon="upload"
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
>
{ __( 'Upload' ) }
</IconButton>
</Button>
{ mediaLibraryButton }
{ this.renderUrlSelectionUI() }
{ this.renderCancelLink() }
Expand Down
6 changes: 0 additions & 6 deletions packages/block-editor/src/components/media-placeholder/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
.block-editor-media-placeholder__url-input-container {
width: 100%;

// Reset the margin to ensure the url popover is adjacent to the button.
.block-editor-media-placeholder__button {
margin-bottom: 0;
Expand Down Expand Up @@ -47,10 +45,6 @@
display: block;
}

.components-form-file-upload .block-editor-media-placeholder__button {
margin-right: $grid-size-small;
}

.block-editor-media-placeholder.is-appender {
min-height: 100px;
outline: $border-width dashed $dark-gray-150;
Expand Down
18 changes: 3 additions & 15 deletions packages/block-library/src/cover/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
color: $light-gray-100;
}

// wp-block-cover class is used just to increase selector specificity
// The .wp-block-cover class is used just to increase selector specificity.
.wp-block-cover__inner-container {
// avoid text align inherit from cover image align.
// Avoid text align inherit from cover image align.
text-align: left;
}

Expand All @@ -24,24 +24,12 @@
margin-right: 0;
}

&.components-placeholder {
// use opacity to work in various editor styles
background: $dark-opacity-light-200;
min-height: 200px;

.is-dark-theme & {
background: $light-opacity-light-200;
}
}

.wp-block-cover__placeholder-background-options {
// wraps about 6 color swatches
max-width: 260px;
margin-top: 1em;
width: 100%;
}

// Apply max-width to floated items that have no intrinsic width
// Apply max-width to floated items that have no intrinsic width.
[data-align="left"] &,
[data-align="right"] & {
max-width: $content-width / 2;
Expand Down
10 changes: 9 additions & 1 deletion packages/block-library/src/embed/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,16 @@

exports[`core/embed block edit matches snapshot 1`] = `
<div
class="components-placeholder wp-block-embed"
class="components-placeholder is-small wp-block-embed"
>
<iframe
aria-hidden="true"
aria-label="resize-listener"
frameborder="0"
src="about:blank"
style="display:block;opacity:0;position:absolute;top:0;left:0;height:100%;width:100%;overflow:hidden;pointer-events:none;z-index:-1"
tabindex="-1"
/>
<div
class="components-placeholder__label"
>
Expand Down
1 change: 0 additions & 1 deletion packages/block-library/src/table/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,6 @@ export class TableEdit extends Component {
label={ __( 'Table' ) }
icon={ <BlockIcon icon={ icon } showColors /> }
instructions={ __( 'Insert a table for sharing data.' ) }
isColumnLayout
>
<form className="wp-block-table__placeholder-form" onSubmit={ this.onCreateTable }>
<TextControl
Expand Down
27 changes: 15 additions & 12 deletions packages/block-library/src/table/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,25 @@
border-style: double;
}

// Extra specificity to override default Placeholder styles.
&__placeholder-form.wp-block-table__placeholder-form {
text-align: left;
align-items: center;
figcaption {
@include caption-style-theme();
}
}

&__placeholder-input {
width: 100px;
}
// Extra specificity to override default Placeholder styles.
.wp-block-table__placeholder-form.wp-block-table__placeholder-form {
.wp-block-table__placeholder-input {
width: $grid-size * 14;
margin-right: $grid-size;
margin-bottom: 0;

&__placeholder-button {
min-width: 100px;
justify-content: center;
input {
height: $icon-button-size;
}
}

figcaption {
@include caption-style-theme();
.wp-block-table__placeholder-button {
margin-top: auto;
margin-right: auto;
}
}
1 change: 1 addition & 0 deletions packages/components/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"mousetrap": "^1.6.2",
"re-resizable": "^6.0.0",
"react-dates": "^17.1.1",
"react-resize-aware": "^3.0.0",
"react-spring": "^8.0.20",
"reakit": "^1.0.0-beta.12",
"rememo": "^3.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/button/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
background: none;
transition: box-shadow 0.1s linear;
@include reduce-motion("transition");
height: 36px;
height: $icon-button-size;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should rename that variable (it's not just about icon) but we should do that separately.

align-items: center;
box-sizing: border-box;
padding: 0 8px;
Expand Down
4 changes: 0 additions & 4 deletions packages/components/src/circular-option-picker/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ $color-palette-circle-spacing: 12px;
height: 100%;
width: 100%;
}

&:nth-child(6n+6) {
margin-right: 0;
}
}

.components-circular-option-picker__option-wrapper::before {
Expand Down
8 changes: 3 additions & 5 deletions packages/components/src/form-file-upload/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Component } from '@wordpress/element';
/**
* Internal dependencies
*/
import IconButton from '../icon-button';
import Button from '../button';

class FormFileUpload extends Component {
constructor() {
Expand All @@ -27,7 +27,6 @@ class FormFileUpload extends Component {
const {
accept,
children,
icon = 'upload',
multiple = false,
onChange,
render,
Expand All @@ -36,13 +35,12 @@ class FormFileUpload extends Component {

const ui = render ?
render( { openFileDialog: this.openFileDialog } ) : (
<IconButton
icon={ icon }
<Button
onClick={ this.openFileDialog }
{ ...props }
>
{ children }
</IconButton>
</Button>
);
return (
<div className="components-form-file-upload">
Expand Down
5 changes: 0 additions & 5 deletions packages/components/src/form-file-upload/style.scss
View file
Open in desktop

This file was deleted.

6 changes: 3 additions & 3 deletions packages/components/src/form-file-upload/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { noop } from 'lodash';
*/
import FormFileUpload from '../';

describe( 'InserterMenu', () => {
describe( 'FormFileUpload', () => {
it( 'should show an Icon Button and a hidden input', () => {
const wrapper = shallow(
<FormFileUpload
Expand All @@ -22,9 +22,9 @@ describe( 'InserterMenu', () => {
</FormFileUpload>
);

const iconButton = wrapper.find( 'ForwardRef(IconButton)' );
const button = wrapper.find( 'ForwardRef(Button)' );
const input = wrapper.find( 'input' );
expect( iconButton.prop( 'children' ) ).toBe( 'My Upload Button' );
expect( button.prop( 'children' ) ).toBe( 'My Upload Button' );
expect( input.prop( 'style' ).display ).toBe( 'none' );
} );
} );
11 changes: 10 additions & 1 deletion packages/components/src/placeholder/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import classnames from 'classnames';
import useResizeAware from 'react-resize-aware';

/**
* Internal dependencies
Expand All @@ -15,10 +16,18 @@ import Icon from '../icon';
* @return {Object} The rendered placeholder.
*/
function Placeholder( { icon, children, label, instructions, className, notices, preview, isColumnLayout, ...additionalProps } ) {
const classes = classnames( 'components-placeholder', className );
const [ resizeListener, { width } ] = useResizeAware();
const classes = classnames(
'components-placeholder',
( width >= 320 ? 'is-large' : '' ),
( width >= 160 && width < 320 ? 'is-medium' : '' ),
( width < 160 ? 'is-small' : '' ),
Copy link
Member

Choose a reason for hiding this comment

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

One thing I note here is that in the documentation of react-resize-aware, it mentions how width would be null the first time the component is rendered:

This object contains the width and height properties, these properties are going to be null before the component rendered, and will return a number after the component rendered.

https://www.npmjs.com/package/react-resize-aware#api

The "fun" thing about null is that, in the context of these sorts of comparisons, it's treated the same as zero (related, specification).

So:

let width = null;
console.log( width < 160 );
// true

So we'll always apply the is-small class on the first render, even if it ends up being that the element will actually occupy a large width.

I guess my question is:

  • Must we have at least one of is-large, is-medium, or is-small, and therefore it would be okay to use is-small as the default until an accurate value can be determined?
  • Otherwise, would it be more correct to wait to assign any of these modifier classes until the true width can be determined?

Copy link
Member

Choose a reason for hiding this comment

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

For context, I think this may be contributing to some intermittent failures in the end-to-end tests, where we're trying to click "Try again" when an embed fails, but the placement of the button might be shifting around because it is rendered after the "instructions" which are hidden while is-small class is applied.

Copy link
Member

Choose a reason for hiding this comment

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

I've opened #19825 for continued discussion.

className
);
const fieldsetClasses = classnames( 'components-placeholder__fieldset', { 'is-column-layout': isColumnLayout } );
return (
<div { ...additionalProps } className={ classes }>
{ resizeListener }
{ notices }
{ preview &&
<div className="components-placeholder__preview">
Expand Down
Loading