Skip to content

Commit

Permalink
Polish menus and focus styles (#6812)
Browse files Browse the repository at this point in the history
* Polish menus and focus styles

This fixes a regression in menu focus styles that was introduced as part of some focus fixes to the movers way back.

It also fixes #6734, as it changes how the shortcuts are spelled out.

* Fix tooltips in menus.

* Simplify keycodes.

* Fix tests.
  • Loading branch information
jasmussen authored May 23, 2018
1 parent 75d3c35 commit 5f3c7f8
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 15 deletions.
7 changes: 4 additions & 3 deletions components/menu-item/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,17 @@
padding-left: 0;
}

&:hover {
&:hover:not(:disabled):not([aria-disabled="true"]) {
@include menu-style__neutral;
}

&:focus {
&:focus:not(:disabled):not([aria-disabled="true"]) {
@include menu-style__focus;
}
}

.components-menu-item__shortcut {
float: right;
opacity: .5;
margin-right: 0;
margin-left: auto;
}
2 changes: 1 addition & 1 deletion components/panel/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
color: $dark-gray-900;
@include menu-style__neutral;

&:focus {
&:focus:not(:disabled):not([aria-disabled="true"]) {
@include menu-style__focus;
}

Expand Down
2 changes: 1 addition & 1 deletion components/popover/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@

.components-popover:not(.is-mobile) & {
position: absolute;
min-width: 240px;
min-width: 260px;
height: auto;
}

Expand Down
5 changes: 2 additions & 3 deletions editor/components/block-settings-menu/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,8 @@
cursor: pointer;
@include menu-style__neutral;

&:hover,
&:focus,
&:not(:disabled):hover {
&:hover:not(:disabled):not([aria-disabled="true"]),
&:focus:not(:disabled):not([aria-disabled="true"]) {
@include menu-style__focus;
}

Expand Down
9 changes: 9 additions & 0 deletions editor/components/block-switcher/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,13 @@
margin-right: 8px;
height: 20px;
}

&:hover:not(:disabled):not([aria-disabled="true"]) {
@include menu-style__neutral;
}

&:focus:not(:disabled):not([aria-disabled="true"]) {
@include menu-style__focus;
}

}
2 changes: 1 addition & 1 deletion editor/components/post-last-revision/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
}

// Needs specificity
.components-icon-button:not(:disabled).editor-post-last-revision__title {
.components-icon-button:not(:disabled):not([aria-disabled="true"]).editor-post-last-revision__title {

&:hover,
&:active {
Expand Down
6 changes: 3 additions & 3 deletions utils/keycodes.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ export const displayShortcut = mapValues( modifiers, ( modifier ) => {
return ( character, _isMac = isMacOS ) => {
const isMac = _isMac();
const replacementKeyMap = {
[ ALT ]: isMac ? '⌥option' : 'Alt',
[ CTRL ]: isMac ? '⌃control' : 'Ctrl',
[ ALT ]: isMac ? 'Option' : 'Alt',
[ CTRL ]: 'Ctrl',
[ COMMAND ]: '⌘',
[ SHIFT ]: isMac ? '⇧shift' : 'Shift',
[ SHIFT ]: 'Shift',
};
const shortcut = [
...modifier( _isMac ).map( ( key ) => get( replacementKeyMap, key, key ) ),
Expand Down
6 changes: 3 additions & 3 deletions utils/test/keycodes.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe( 'displayShortcut', () => {

it( 'should output shift+command symbols on MacOS', () => {
const shortcut = displayShortcut.primaryShift( 'm', isMacOSTrue );
expect( shortcut ).toEqual( '⇧shift+⌘M' );
expect( shortcut ).toEqual( 'Shift+⌘M' );
} );
} );

Expand All @@ -43,7 +43,7 @@ describe( 'displayShortcut', () => {

it( 'should output shift+option+command symbols on MacOS', () => {
const shortcut = displayShortcut.secondary( 'm', isMacOSTrue );
expect( shortcut ).toEqual( '⇧shift+⌥option+⌘M' );
expect( shortcut ).toEqual( 'Shift+Option+⌘M' );
} );
} );

Expand All @@ -55,7 +55,7 @@ describe( 'displayShortcut', () => {

it( 'should output control+option symbols on MacOS', () => {
const shortcut = displayShortcut.access( 'm', isMacOSTrue );
expect( shortcut ).toEqual( '⌃control+⌥option+M' );
expect( shortcut ).toEqual( 'Ctrl+Option+M' );
} );
} );
} );
Expand Down

0 comments on commit 5f3c7f8

Please sign in to comment.