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

Refactor the MediaReplaceFlow component to use Dropdown #19126

Merged
merged 6 commits into from
Dec 16, 2019
+83 −129
Merged

Refactor the MediaReplaceFlow component to use Dropdown #19126

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
de6766c
Refactor the MediaReplaceFlow component to use Dropdown
youknowriad Dec 13, 2019
8f2281e
Use ToolbarGroup
youknowriad Dec 16, 2019
42cd31d
Fix classnames
youknowriad Dec 16, 2019
c53e7e9
Fix some styling issues
youknowriad Dec 16, 2019
7856e68
Fix z-index issue
youknowriad Dec 16, 2019
88afc15
Fix input width
youknowriad Dec 16, 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
115 changes: 54 additions & 61 deletions packages/block-editor/src/components/media-replace-flow/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
MenuItem,
Toolbar,
Button,
Popover,
Dropdown,
withNotices,
} from '@wordpress/components';
import {
Expand Down Expand Up @@ -46,7 +46,6 @@ const MediaReplaceFlow = (
const [ showURLInput, setShowURLInput ] = useState( false );
const [ showEditURLInput, setShowEditURLInput ] = useState( false );
const [ mediaURLValue, setMediaURLValue ] = useState( mediaURL );
const [ showMediaReplaceOptions, setShowMediaReplaceOptions ] = useState( false );
const mediaUpload = useSelect( ( select ) => {
return select( 'core/block-editor' ).getSettings().mediaUpload;
}, [] );
Expand Down Expand Up @@ -74,11 +73,11 @@ const MediaReplaceFlow = (
setShowEditURLInput( false );
};

const uploadFiles = ( event ) => {
const uploadFiles = ( event, closeDropdown ) => {
const files = event.target.files;
const setMedia = ( [ media ] ) => {
setShowMediaReplaceOptions( false );
selectMedia( media );
closeDropdown();
};
mediaUpload( {
allowedTypes,
Expand All @@ -88,12 +87,6 @@ const MediaReplaceFlow = (
} );
};

const onClose = () => {
editMediaButtonRef.current.focus();
};

const onClickOutside = () => ( setShowMediaReplaceOptions( false ) );

const openOnArrowDown = ( event ) => {
if ( event.keyCode === DOWN ) {
event.preventDefault();
Expand Down Expand Up @@ -131,68 +124,68 @@ const MediaReplaceFlow = (
}

return (
<MediaUpload
onSelect={ ( media ) => selectMedia( media ) }
onClose={ () => setShowMediaReplaceOptions( true ) }
allowedTypes={ allowedTypes }
render={ ( { open } ) => (
<Toolbar className={ 'media-replace-flow components-dropdown-menu' }>
<Dropdown
Copy link
Member

Choose a reason for hiding this comment

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

There is also this DropdownMenu component. Ou of curiosity, why isn’t it a good fit? We clearly need a ToolbarGroup which is collapsible and renders as button with menu. @diegohaz started some explorations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason it's not a DropdownMenu is because it contains elements that are not menu items (the URL input). There's some discussion about this in the original PR.

Copy link
Member

Choose a reason for hiding this comment

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

Right, I'm personally wondering whether there should be any menu items used in this popover in that case. It might be a bit confusing for VoiceOver users.

contentClassName="media-replace-flow__options"
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
renderToggle={ ( { isOpen, onToggle } ) => (
<Toolbar className="media-replace-flow">
draganescu marked this conversation as resolved.
Show resolved Hide resolved
<Button
ref={ editMediaButtonRef }
className={ 'components-icon-button components-dropdown-menu__toggle' }
onClick={ () => {
setShowMediaReplaceOptions( ! showMediaReplaceOptions );
} }
aria-expanded={ isOpen }
onClick={ onToggle }
onKeyDown={ openOnArrowDown }
>
<span className="components-dropdown-menu__label" > { name } </span>
<span className="components-dropdown-menu__indicator" />
{ name }
<span className="media-replace-flow__indicator" />
</Button>
{ showMediaReplaceOptions &&
<Popover
onClickOutside={ onClickOutside }
onClose={ onClose }
className={ 'media-replace-flow__options' }
>
<NavigableMenu>
</Toolbar>
) }
renderContent={ ( { onClose } ) => (
<>
<NavigableMenu>
<MediaUpload
onSelect={ ( media ) => selectMedia( media ) }
allowedTypes={ allowedTypes }
render={ ( { open } ) => (
<MenuItem
icon="admin-media"
onClick={ open }
>
{ __( 'Open Media Library' ) }
</MenuItem>
<MediaUploadCheck>
<FormFileUpload
onChange={ uploadFiles }
accept={ accept }
render={ ( { openFileDialog } ) => {
return (
<MenuItem
icon="upload"
onClick={ () => {
openFileDialog();
} }
>
{ __( 'Upload' ) }
</MenuItem>
);
} }
/>
</MediaUploadCheck>
<MenuItem
icon="admin-links"
onClick={ () => ( setShowURLInput( ! showURLInput ) ) }
aria-expanded={ showURLInput }
>
<div> { __( 'Insert from URL' ) } </div>
</MenuItem>
</NavigableMenu>
{ showURLInput && <div className="block-editor-media-flow__url-input">
{ urlInputUIContent }
</div> }
</Popover>
}
</Toolbar>
) }
/>
<MediaUploadCheck>
<FormFileUpload
onChange={ ( event ) => {
uploadFiles( event, onClose );
} }
accept={ accept }
render={ ( { openFileDialog } ) => {
return (
<MenuItem
icon="upload"
onClick={ () => {
openFileDialog();
} }
>
{ __( 'Upload' ) }
</MenuItem>
);
} }
/>
</MediaUploadCheck>
<MenuItem
icon="admin-links"
onClick={ () => ( setShowURLInput( ! showURLInput ) ) }
aria-expanded={ showURLInput }
>
<div> { __( 'Insert from URL' ) } </div>
</MenuItem>
</NavigableMenu>
{ showURLInput && <div className="block-editor-media-flow__url-input">
{ urlInputUIContent }
</div> }
</>
) }
/>
);
Expand Down
95 changes: 41 additions & 54 deletions packages/block-editor/src/components/media-replace-flow/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,75 +1,62 @@
.media-replace-flow .components-dropdown-menu__indicator {
.media-replace-flow__indicator {
margin-left: 4px;

.components-dropdown-menu.media-flow_toolbar {
.components-dropdown-menu__label {
margin-right: 6px;
margin-left: 2px;
}
&::after {
@include dropdown-arrow();
}
}

.media-replace-flow__options.components-popover:not(.is-mobile) {
.block-editor-media-flow__url-input {
padding: 0 15px 10px 25px;

.components-popover__content {
// this is a safari problem that shows scrollbars
// when display:flex and max-width are used together
overflow-x: hidden;
.components-external-link__icon {
position: absolute;
right: -4px;
bottom: 5px;
margin-right: 2px;
}

.block-editor-media-flow__url-input {
input {
max-width: 169px;
border: 1px solid $dark-gray-500;
border-radius: 4px;
}

padding: 0 15px 10px 25px;
.block-editor-url-popover__link-viewer-url {
padding-right: 15px;
padding-top: 3px;
max-width: 179px;
position: relative;
margin-right: 0;
}

.components-external-link__icon {
position: absolute;
right: -4px;
bottom: 5px;
margin-right: 2px;
}
// Mimic toolbar component styles for the icons in this popover.
.components-icon-button {
padding: 5px;
width: 40px;
height: 40px;
padding-left: 0;

input {
max-width: 169px;
border: 1px solid $dark-gray-500;
border-radius: 4px;
> svg {
padding: 5px;
border-radius: $radius-round-rectangle;
height: 30px;
width: 30px;
}

.block-editor-url-popover__link-viewer-url {
padding-right: 15px;
padding-top: 3px;
max-width: 179px;
position: relative;
margin-right: 0;
}

// Mimic toolbar component styles for the icons in this popover.
.components-icon-button {
padding: 5px;
width: 40px;
height: 40px;
padding-left: 0;
&:not(:disabled):not([aria-disabled="true"]):not(.is-secondary):hover {
box-shadow: none;

> svg {
padding: 5px;
border-radius: $radius-round-rectangle;
height: 30px;
width: 30px;
}

&:not(:disabled):not([aria-disabled="true"]):not(.is-secondary):hover {
box-shadow: none;

> svg {
@include formatting-button-style__hover;
}
@include formatting-button-style__hover;
}
}

&:not(:disabled):focus {
box-shadow: none;
&:not(:disabled):focus {
box-shadow: none;

> svg {
@include formatting-button-style__focus;
}
> svg {
@include formatting-button-style__focus;
}
}
}
Expand Down