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

Inserter: Clarify that when the Inserter is open clicking the + button in the top bar will close it again #29759

Merged
merged 10 commits into from
Mar 31, 2021
Merged

Inserter: Clarify that when the Inserter is open clicking the + button in the top bar will close it again #29759

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b118325
Rotate the inserter icon and switch the label when pressed
jameskoster Mar 11, 2021
a680c1d
Use a single label rather than toggling. Include reduce-motion mixin.
jameskoster Mar 17, 2021
9dbcc74
Update tests
jameskoster Mar 18, 2021
e3a8268
formatting
jameskoster Mar 18, 2021
dd59a7f
Update test snapshots
jameskoster Mar 18, 2021
d884c1a
Merge branch 'trunk' into try/close-inserter-button
jameskoster Mar 19, 2021
7e38d5b
Merge branch 'trunk' into try/close-inserter-button
jameskoster Mar 22, 2021
128b6ed
Update labels
jameskoster Mar 26, 2021
94264f7
Add old selectors
david-szabo97 Mar 29, 2021
944fd6c
Add comment
david-szabo97 Mar 29, 2021
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
4 changes: 2 additions & 2 deletions packages/e2e-test-utils/src/inserter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export async function closeGlobalBlockInserter() {
async function isGlobalInserterOpen() {
return await page.evaluate( () => {
return !! document.querySelector(
'.edit-post-header [aria-label="Add block"].is-pressed, .edit-site-header [aria-label="Add block"].is-pressed'
'.edit-post-header [aria-label="Add block"].is-pressed, .edit-site-header [aria-label="Add block"].is-pressed, .edit-post-header [aria-label="Toggle block inserter"].is-pressed, .edit-site-header [aria-label="Toggle block inserter"].is-pressed'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a comment explaining why we need both, so future contributors can make good decisions about when to remove.

);
} );
}
Expand All @@ -50,7 +50,7 @@ async function isGlobalInserterOpen() {
*/
export async function toggleGlobalBlockInserter() {
await page.click(
'.edit-post-header [aria-label="Add block"], .edit-site-header [aria-label="Add block"]'
'.edit-post-header [aria-label="Add block"], .edit-site-header [aria-label="Add block"], .edit-post-header [aria-label="Toggle block inserter"], .edit-site-header [aria-label="Toggle block inserter"]'
);
}

Expand Down
6 changes: 4 additions & 2 deletions packages/e2e-tests/specs/editor/plugins/cpt-locking.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe( 'cpt locking', () => {
expect(
await page.evaluate( () => {
const inserter = document.querySelector(
'.edit-post-header [aria-label="Add block"]'
'.edit-post-header [aria-label="Add block"], .edit-post-header [aria-label="Toggle block inserter"]'
);
return inserter.getAttribute( 'disabled' );
} )
Expand Down Expand Up @@ -159,7 +159,9 @@ describe( 'cpt locking', () => {

it( 'should allow blocks to be inserted', async () => {
expect(
await page.$( '.edit-post-header [aria-label="Add block"]' )
await page.$(
'.edit-post-header [aria-label="Add block"], .edit-post-header [aria-label="Toggle block inserter"]'
)
).not.toBeNull();
await insertBlock( 'List' );
await page.keyboard.type( 'List content' );
Expand Down
2 changes: 1 addition & 1 deletion 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
Expand Up @@ -143,7 +143,7 @@ function HeaderToolbar() {
/* translators: button label text should, if possible, be under 16
characters. */
label={ _x(
'Add block',
'Toggle block inserter',
'Generic label for block inserter button'
) }
showTooltip={ ! showIconLabels }
Expand Down
11 changes: 11 additions & 0 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 @@ -15,6 +15,17 @@

.edit-post-header-toolbar__left > .edit-post-header-toolbar__inserter-toggle {
display: inline-flex;

svg {
transition: transform cubic-bezier(0.165, 0.84, 0.44, 1) 0.2s;
david-szabo97 marked this conversation as resolved.
Show resolved Hide resolved
@include reduce-motion("transition");
}

&.is-pressed {
svg {
transform: rotate(45deg);
Copy link
Contributor

Choose a reason for hiding this comment

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

If we go and keep this, it might be nice to update the Figma "small x" icon to be more or less identical to the plus as rotated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, let's assess in a follow up.

Copy link
Contributor

Choose a reason for hiding this comment

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

(It might already be!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked, it's close but no cigar 😅

Screenshot 2021-03-17 at 15 01 19

Copy link
Contributor

Choose a reason for hiding this comment

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

Hah, I bet that when the pixels antialias together the difference is barely perceptible.

Of course that won't be good enough for you or me, because we'll know! But I'm happy to push the vectors once we get to it 😄

}
}
}

// Hide table of contents and block navigation on mobile.
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-site/src/components/header/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export default function Header( { openEntitiesSavedStates } ) {
} }
icon={ plus }
label={ _x(
'Add block',
'Toggle block inserter',
'Generic label for block inserter button'
) }
/>
Expand Down
11 changes: 11 additions & 0 deletions packages/edit-site/src/components/header/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,17 @@ body.is-navigation-sidebar-open {
width: $grid-unit-40;
height: $grid-unit-40;
padding: 0;

svg {
transition: transform cubic-bezier(0.165, 0.84, 0.44, 1) 0.2s;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as before, include the mixin.

@include reduce-motion("transition");
}

&.is-pressed {
svg {
transform: rotate(45deg);
}
}
}
}

Expand Down