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

Added show icon labels option #15830

Closed
wants to merge 24 commits into from
+318 −81
Closed

Added show icon labels option #15830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d8b4250
Small cleanup.
nicolad Mar 19, 2020
7aa7f88
Updated HeaderToolbar.
nicolad Mar 19, 2020
8370fa3
Conflicts fixed.
nicolad Apr 8, 2020
daaec4f
Updated MoreMenu and it's styles.
nicolad Mar 19, 2020
64c452a
Updated Header style.scss.
nicolad Mar 19, 2020
41a6456
Updated OptionsModal.
nicolad Mar 19, 2020
2685980
Updated PREFERENCES_DEFAULTS.
nicolad Mar 19, 2020
3508c61
Updated HeaderToolbar.
nicolad Mar 19, 2020
4d127fd
Updated Header.
nicolad Mar 19, 2020
2a96c72
Snapshots update.
nicolad Mar 19, 2020
585b81a
Small improvement; js_only.
nicolad Mar 19, 2020
2ef2d38
Shortened MoreMenu label.
nicolad Mar 20, 2020
c66a1ea
Applied black color to the label related to primary button.
nicolad Mar 20, 2020
ce28dbc
Small cleanup; js_only.
nicolad Mar 20, 2020
477fad7
Removed overflow: hidden; on components-button class name.
nicolad Mar 23, 2020
941a0bb
Updated HeaderToolbar and it's styles.
nicolad Apr 8, 2020
229addc
Moved header settings into a dropdown when viewport is less than large.
nicolad Apr 12, 2020
fee19fd
Conflicts fixed.
nicolad Apr 24, 2020
8478475
Hide tooltip when show icon is enabled.
nicolad Apr 24, 2020
2fcd682
Merge branch 'master' into enhancement/show-icon-labels
nicolad May 9, 2020
fac54fa
Small improvement; js_only.
nicolad May 9, 2020
90595aa
Dropdown styles adjusted.
nicolad May 9, 2020
bccbab5
Merge branch 'master' into enhancement/show-icon-labels
nicolad May 17, 2020
ea00342
Dropdown styles adjusted.
nicolad May 17, 2020
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: 9 additions & 1 deletion packages/block-editor/src/components/block-navigation/dropdown.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ const MenuIcon = (
</SVG>
);

function BlockNavigationDropdownToggle( { isEnabled, onToggle, isOpen } ) {
function BlockNavigationDropdownToggle( {
isEnabled,
onToggle,
isOpen,
showTooltip,
} ) {
useShortcut(
'core/edit-post/toggle-block-navigation',
useCallback( onToggle, [ onToggle ] ),
Expand All @@ -49,12 +54,14 @@ function BlockNavigationDropdownToggle( { isEnabled, onToggle, isOpen } ) {
className="block-editor-block-navigation"
shortcut={ shortcut }
aria-disabled={ ! isEnabled }
showTooltip={ showTooltip }
/>
);
}

function BlockNavigationDropdown( {
isDisabled,
showTooltip,
__experimentalWithBlockNavigationSlots,
} ) {
const hasBlocks = useSelect(
Expand All @@ -71,6 +78,7 @@ function BlockNavigationDropdown( {
<BlockNavigationDropdownToggle
{ ...toggleProps }
isEnabled={ isEnabled }
showTooltip={ showTooltip }
/>
) }
renderContent={ ( { onClose } ) => (
Expand Down
3 changes: 2 additions & 1 deletion packages/block-editor/src/components/tool-selector/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const selectIcon = (
</SVG>
);

function ToolSelector() {
function ToolSelector( { showTooltip } ) {
const isNavigationTool = useSelect(
( select ) => select( 'core/block-editor' ).isNavigationMode(),
[]
Expand All @@ -57,6 +57,7 @@ function ToolSelector() {
aria-expanded={ isOpen }
onClick={ onToggle }
label={ __( 'Tools' ) }
showTooltip={ showTooltip }
/>
) }
position="bottom right"
Expand Down
5 changes: 3 additions & 2 deletions packages/components/src/button/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export function Button( props, ref ) {
disabled,
icon,
iconSize,
showIconLabel,
showTooltip,
tooltipPosition,
shortcut,
Expand Down Expand Up @@ -94,8 +95,7 @@ export function Button( props, ref ) {
! trulyDisabled &&
// an explicit tooltip is passed or...
( ( showTooltip && label ) ||
// there's a shortcut or...
shortcut ||
( shortcut && showTooltip !== false ) ||
// there's a label and...
( !! label &&
// the children are empty and...
Expand All @@ -113,6 +113,7 @@ export function Button( props, ref ) {
ref={ ref }
>
{ icon && <Icon icon={ icon } size={ iconSize } /> }
{ showIconLabel && <div className="label">{ label }</div> }
{ children }
</Tag>
);
Expand Down
6 changes: 5 additions & 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 @@ -13,7 +13,6 @@
align-items: center;
box-sizing: border-box;
padding: 6px 12px;
overflow: hidden;
Copy link
Member Author

Choose a reason for hiding this comment

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

this was removed based on this comment: #19058 (comment)
cc @tellthemachines

border-radius: $radius-block-ui;
color: $dark-gray-primary;

Expand Down Expand Up @@ -280,6 +279,11 @@
outline: none;
}

.label {
flex-basis: 100%;
margin-left: 5px;
}

// Fixes a Safari+VoiceOver bug, where the screen reader text is announced not respecting the source order.
// See https://core.trac.wordpress.org/ticket/42006 and https://github.com/h5bp/html5-boilerplate/issues/1985
.components-visually-hidden {
Expand Down
8 changes: 6 additions & 2 deletions packages/components/src/dropdown-menu/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import classnames from 'classnames';
import { flatMap, isEmpty, isFunction } from 'lodash';
import { flatMap, isEmpty, isFunction, isUndefined } from 'lodash';

/**
* WordPress dependencies
Expand Down Expand Up @@ -124,7 +124,11 @@ function DropdownMenu( {
aria-haspopup="true"
aria-expanded={ isOpen }
label={ label }
showTooltip
showTooltip={
isUndefined( mergedToggleProps.showTooltip )
? true
: mergedToggleProps.showTooltip
}
>
{ mergedToggleProps.children }
</Button>
Expand Down
33 changes: 27 additions & 6 deletions packages/edit-post/src/components/header/header-toolbar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
Expand Down Expand Up @@ -25,6 +30,7 @@ function HeaderToolbar( { onToggleInserter, isInserterOpen } ) {
isInserterVisible,
isTextModeEnabled,
previewDeviceType,
showIconLabels,
} = useSelect(
( select ) => ( {
hasFixedToolbar: select( 'core/edit-post' ).isFeatureActive(
Expand All @@ -40,14 +46,20 @@ function HeaderToolbar( { onToggleInserter, isInserterOpen } ) {
previewDeviceType: select(
'core/edit-post'
).__experimentalGetPreviewDeviceType(),
showIconLabels: select( 'core/edit-post' ).isFeatureActive(
'showIconLabels'
),
} ),
[]
);
const isLargeViewport = useViewportMatch( 'medium' );
const isWideViewport = useViewportMatch( 'wide' );

const displayBlockToolbar =
! isLargeViewport || previewDeviceType !== 'Desktop' || hasFixedToolbar;

const showTooltip = ! showIconLabels;

const toolbarAriaLabel = displayBlockToolbar
? /* translators: accessibility text for the editor toolbar when Top Toolbar is on */
__( 'Document and block tools' )
Expand All @@ -56,8 +68,10 @@ function HeaderToolbar( { onToggleInserter, isInserterOpen } ) {

return (
<NavigableToolbar
className="edit-post-header-toolbar"
aria-label={ toolbarAriaLabel }
className={ classnames( 'edit-post-header-toolbar', {
'show-icon-labels': showIconLabels,
} ) }
>
{ isInserterVisible && (
<Button
Expand All @@ -71,13 +85,20 @@ function HeaderToolbar( { onToggleInserter, isInserterOpen } ) {
'Add block',
'Generic label for block inserter button'
) }
showTooltip={ showTooltip }
/>
) }
<ToolSelector />
<EditorHistoryUndo />
<EditorHistoryRedo />
<TableOfContents hasOutlineItemsDisabled={ isTextModeEnabled } />
<BlockNavigationDropdown isDisabled={ isTextModeEnabled } />
<ToolSelector showTooltip={ showTooltip } />
<EditorHistoryUndo showTooltip={ showTooltip } />
<EditorHistoryRedo showTooltip={ showTooltip } />
<TableOfContents
hasOutlineItemsDisabled={ isTextModeEnabled }
showTooltip={ showTooltip }
/>
<BlockNavigationDropdown
isDisabled={ isTextModeEnabled }
showTooltip={ showTooltip }
/>
{ displayBlockToolbar && (
<div className="edit-post-header-toolbar__block-toolbar">
<BlockToolbar hideDragHandle />
Expand Down
31 changes: 29 additions & 2 deletions packages/edit-post/src/components/header/header-toolbar/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,34 @@
display: flex;
}
}
&.show-icon-labels {
button[aria-label] {
display: inline-flex;
flex-wrap: nowrap;
&::after {
content: attr(aria-label);
Copy link
Member

Choose a reason for hiding this comment

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

This is an interesting approach. I like it keeps the components relatively clean.

Copy link
Member

Choose a reason for hiding this comment

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

I'd like to get some A11y feedback on this, as support for content in CSS differs a bit among assistive technologies. We don't wanna end up with a screen reader reading the label out loud twice.

Copy link
Member

Choose a reason for hiding this comment

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

Indeed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I found that @afercia already had an opinion on this: #10524 (comment)
Will change the approach to meet a11y requirements.

Copy link
Contributor

Choose a reason for hiding this comment

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

@swissspidy thanks Pascal for raising this concern. You're right that some screen readers read out CSS generated content but in this case the whole content of the buttons is overridden by the aria-label so that only the aria-label is announced. To be 100% sure, I tested it also with NVDA and JAWS on Windows Firefox / Chrome.

Copy link
Member

@gziolo gziolo Jun 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Wouldn't it be easier to inline this label inside HTML when showIconLabels prop is set to true? it also feels like injecting showIconLabels prop should be part of the IconButton component which is the one which actually renders those icons.

display: block;
font-size: 12px;
margin-left: 5px;
}
&.is-primary {
&::after {
padding-right: 8px;
}
}
}
[aria-orientation=vertical] & {
display: block;
}
> div, > button {
[aria-orientation=vertical] & {
display: block;
button {
width: 100%;
}
}
}
}
}

// Block toolbar when fixed to the top of the screen.
Expand Down Expand Up @@ -75,7 +103,7 @@

.block-editor-block-toolbar .components-toolbar-group,
.block-editor-block-toolbar .components-toolbar {
$top-toolbar-padding: ($header-height - $grid-unit-60) / 2;
$top-toolbar-padding: ( $header-height - $grid-unit-60 ) / 2;
height: $header-height;
padding: $top-toolbar-padding 0;
}
Expand All @@ -86,7 +114,6 @@
margin-right: $grid-unit-10;
// Special dimensions for this button.
min-width: 32px;
width: 32px;
height: 32px;
padding: 0;
}
101 changes: 65 additions & 36 deletions packages/edit-post/src/components/header/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { Button } from '@wordpress/components';
import { Button, Dropdown } from '@wordpress/components';
import { PostSavedState, PostPreviewButton } from '@wordpress/editor';
import { useSelect, useDispatch } from '@wordpress/data';
import { cog } from '@wordpress/icons';
import { useViewportMatch } from '@wordpress/compose';
import { cog, moreHorizontal } from '@wordpress/icons';
import {
PinnedItems,
__experimentalMainDashboardButton as MainDashboardButton,
Expand All @@ -32,6 +33,7 @@ function Header( {
isPublishSidebarOpened,
isSaving,
getBlockSelectionStart,
showIconLabel,
} = useSelect(
( select ) => ( {
shortcut: select(
Expand All @@ -48,6 +50,9 @@ function Header( {
getBlockSelectionStart: select( 'core/block-editor' )
.getBlockSelectionStart,
isPostSaveable: select( 'core/editor' ).isEditedPostSaveable(),
showIconLabel: select( 'core/edit-post' ).isFeatureActive(
'showIconLabels'
),
deviceType: select(
'core/edit-post'
).__experimentalGetPreviewDeviceType(),
Expand All @@ -58,6 +63,8 @@ function Header( {
'core/edit-post'
);

const isLargeViewport = useViewportMatch( 'large' );

const toggleGeneralSidebar = isEditorSidebarOpened
? closeGeneralSidebar
: () =>
Expand All @@ -67,6 +74,46 @@ function Header( {
: 'edit-post/document'
);

const headerSettings = (
<div className="edit-post-header__settings">
{ ! isPublishSidebarOpened && (
// This button isn't completely hidden by the publish sidebar.
// We can't hide the whole toolbar when the publish sidebar is open because
// we want to prevent mounting/unmounting the PostPublishButtonOrToggle DOM node.
// We track that DOM node to return focus to the PostPublishButtonOrToggle
// when the publish sidebar has been closed.
<PostSavedState
forceIsDirty={ hasActiveMetaboxes }
forceIsSaving={ isSaving }
/>
) }
<DevicePreview />
<PostPreviewButton
forceIsAutosaveable={ hasActiveMetaboxes }
forcePreviewLink={ isSaving ? null : undefined }
/>
<PostPublishButtonOrToggle
forceIsDirty={ hasActiveMetaboxes }
forceIsSaving={ isSaving }
setEntitiesSavedStatesCallback={
setEntitiesSavedStatesCallback
}
/>
<Button
icon={ cog }
label={ __( 'Settings' ) }
onClick={ toggleGeneralSidebar }
isPressed={ isEditorSidebarOpened }
aria-expanded={ isEditorSidebarOpened }
showIconLabel={ showIconLabel }
showTooltip={ ! showIconLabel }
shortcut={ shortcut }
/>
<PinnedItems.Slot scope="core/edit-post" />
<MoreMenu showIconLabel={ showIconLabel } />
</div>
);

return (
<div className="edit-post-header">
<MainDashboardButton.Slot>
Expand All @@ -78,41 +125,23 @@ function Header( {
isInserterOpen={ isInserterOpen }
/>
</div>
<div className="edit-post-header__settings">
{ ! isPublishSidebarOpened && (
// This button isn't completely hidden by the publish sidebar.
// We can't hide the whole toolbar when the publish sidebar is open because
// we want to prevent mounting/unmounting the PostPublishButtonOrToggle DOM node.
// We track that DOM node to return focus to the PostPublishButtonOrToggle
// when the publish sidebar has been closed.
<PostSavedState
forceIsDirty={ hasActiveMetaboxes }
forceIsSaving={ isSaving }
/>
) }
<DevicePreview />
<PostPreviewButton
forceIsAutosaveable={ hasActiveMetaboxes }
forcePreviewLink={ isSaving ? null : undefined }
/>
<PostPublishButtonOrToggle
forceIsDirty={ hasActiveMetaboxes }
forceIsSaving={ isSaving }
setEntitiesSavedStatesCallback={
setEntitiesSavedStatesCallback
}
/>
<Button
icon={ cog }
label={ __( 'Settings' ) }
onClick={ toggleGeneralSidebar }
isPressed={ isEditorSidebarOpened }
aria-expanded={ isEditorSidebarOpened }
shortcut={ shortcut }
{ isLargeViewport && headerSettings }
{ ! isLargeViewport && (
<Dropdown
className="my-container-class-name"
contentClassName="my-popover-content-classname"
position="bottom right"
renderToggle={ ( { isOpen, onToggle } ) => (
<Button
aria-expanded={ isOpen }
icon={ moreHorizontal }
isSecondary
onClick={ onToggle }
/>
) }
renderContent={ () => headerSettings }
/>
<PinnedItems.Slot scope="core/edit-post" />
<MoreMenu />
</div>
) }
</div>
);
}
Expand Down
Loading