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.

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

Tabs: unify vertical tabs styles #65387

Merged
merged 61 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
1fe9f0b
Remove inserter pattern overrides
DaniGuardiola Sep 17, 2024
aa7e9d9
Merge branch 'trunk' of https://github.com/WordPress/gutenberg into i…
DaniGuardiola Sep 19, 2024
a3e54c5
Make font weights 400 (inactive) and 500 (active)
DaniGuardiola Sep 19, 2024
0b436e8
Apply styles only when vertical.
DaniGuardiola Sep 20, 2024
299d23c
Make vertical indicator theme accent color at 4% opacity.
DaniGuardiola Sep 20, 2024
30cda58
Make height 48px.
DaniGuardiola Sep 24, 2024
4749518
Add radius.
DaniGuardiola Sep 24, 2024
4ba287f
Also use hover styles in focus-visible.
DaniGuardiola Sep 24, 2024
7847ed6
Fix indicator not visible in inserter > patterns/media.
DaniGuardiola Sep 27, 2024
c9e8020
Adjust padding.
DaniGuardiola Sep 27, 2024
0499718
Tweak focus ring.
DaniGuardiola Sep 27, 2024
77ce702
Wrap long labels.
DaniGuardiola Sep 27, 2024
23f5438
Add chevron and fix a few minor details.
DaniGuardiola Sep 29, 2024
5ffc33d
Merge branch 'trunk' of https://github.com/WordPress/gutenberg into i…
DaniGuardiola Sep 30, 2024
fa2da0a
Fix merge issues.
DaniGuardiola Sep 30, 2024
af857d3
Fix focus indicator (gets cropped with the new overflow auto setting)
DaniGuardiola Sep 30, 2024
a8a3f3f
Merge branch 'trunk' of https://github.com/WordPress/gutenberg into i…
DaniGuardiola Sep 30, 2024
9867089
Fix unwanted chevron.
DaniGuardiola Oct 1, 2024
bd0a96c
Merge branch 'trunk' of https://github.com/WordPress/gutenberg into i…
DaniGuardiola Oct 2, 2024
56989af
Fix unwanted nested scrollbar in inserter > patterns/media vertical t…
DaniGuardiola Oct 2, 2024
517950e
Switch to transform for performance.
DaniGuardiola Oct 2, 2024
a708e6b
Adjust border-radius based on scaling factor.
DaniGuardiola Oct 2, 2024
4aa21c8
Apply feedback.
DaniGuardiola Oct 2, 2024
dcbd6f5
Add changelog entry.
DaniGuardiola Oct 3, 2024
0a65888
Merge branch 'trunk' of https://github.com/WordPress/gutenberg into i…
DaniGuardiola Oct 3, 2024
f0902e8
Switch to `padding-inline`.
DaniGuardiola Oct 3, 2024
e12afb5
Merge branch 'trunk' of https://github.com/WordPress/gutenberg into i…
DaniGuardiola Oct 3, 2024
535463c
Remove unnecessary styles.
DaniGuardiola Oct 4, 2024
3c43be8
Fix horizontal tabs height.
DaniGuardiola Oct 4, 2024
2d4bc91
Remove more unnecessary styles (padding).
DaniGuardiola Oct 4, 2024
7f1e764
Make horizontal padding specific to inline.
DaniGuardiola Oct 4, 2024
34cece9
Make flex/whitespace styles more explicit.
DaniGuardiola Oct 4, 2024
3b82ba0
Make scroll margin specific to vertical tabs.
DaniGuardiola Oct 4, 2024
789545f
The "inline" in inline-flex is unnecessary and confusing, removed it.
DaniGuardiola Oct 4, 2024
094786c
Remove unnecessary position: relative
DaniGuardiola Oct 4, 2024
87ce756
Make resets more explicit
DaniGuardiola Oct 4, 2024
9808c75
Remove unnecessary text-align.
DaniGuardiola Oct 4, 2024
74851f8
Improve comment
DaniGuardiola Oct 4, 2024
cba3d80
Remove unnecessary margin-left
DaniGuardiola Oct 4, 2024
e6b902f
Clean up TabList styles.
DaniGuardiola Oct 4, 2024
41dc764
Adjust text-align.
DaniGuardiola Oct 4, 2024
cfc7928
Clean up selector
DaniGuardiola Oct 4, 2024
d345ebd
Fix focus indicator
DaniGuardiola Oct 4, 2024
9c28772
Clean up position: relative.
DaniGuardiola Oct 4, 2024
f831078
Fix typo.
DaniGuardiola Oct 4, 2024
b4f5c21
Add position: relative back.
DaniGuardiola Oct 4, 2024
a9b1578
Improve focus indicator when selectOnMove is enabled.
DaniGuardiola Oct 4, 2024
5736e59
Merge branch 'trunk' of https://github.com/WordPress/gutenberg into i…
DaniGuardiola Oct 4, 2024
f4dbf4f
Add fade in effect to chevron when selectOnMove is enabled.
DaniGuardiola Oct 4, 2024
94f788b
Use [data-focus-visible] consistently.
DaniGuardiola Oct 4, 2024
01f31da
Styles clean up.
DaniGuardiola Oct 4, 2024
4337d49
Add comment for clarity.
DaniGuardiola Oct 4, 2024
01cefc9
Move scroll-margin to the right place.
DaniGuardiola Oct 4, 2024
bb6b4e1
Use CSS variable for accuracy.
DaniGuardiola Oct 4, 2024
b6710f2
Fix overflow.
DaniGuardiola Oct 4, 2024
822715b
Skip failing test for Safari :(
DaniGuardiola Oct 4, 2024
ece5efc
Fix flashing issue.
DaniGuardiola Oct 4, 2024
07069c8
Transition chevron only on selected and not on hover or focus-visible.
DaniGuardiola Oct 4, 2024
b008f3a
Improve chevron opacity transition with suggested value.
DaniGuardiola Oct 7, 2024
ee8783f
fix changelog
DaniGuardiola Oct 7, 2024
3d9bde2
Merge branch 'trunk' of https://github.com/WordPress/gutenberg into i…
DaniGuardiola Oct 7, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,10 @@
* WordPress dependencies
*/
import { usePrevious, useReducedMotion } from '@wordpress/compose';
import { isRTL } from '@wordpress/i18n';
import {
__experimentalHStack as HStack,
FlexBlock,
privateApis as componentsPrivateApis,
__unstableMotion as motion,
} from '@wordpress/components';
import { Icon, chevronRight, chevronLeft } from '@wordpress/icons';

/**
* Internal dependencies
Expand Down Expand Up @@ -55,18 +51,12 @@ function CategoryTabs( {
<Tabs.Tab
key={ category.name }
tabId={ category.name }
className="block-editor-inserter__category-tab"
aria-label={ category.label }
aria-current={
category === selectedCategory ? 'true' : undefined
}
>
<HStack>
<FlexBlock>{ category.label }</FlexBlock>
<Icon
icon={ isRTL() ? chevronLeft : chevronRight }
/>
</HStack>
{ category.label }
</Tabs.Tab>
) ) }
</Tabs.TabList>
Expand Down
42 changes: 1 addition & 41 deletions packages/block-editor/src/components/inserter/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -214,55 +214,15 @@ $block-inserter-tabs-height: 44px;

.block-editor-inserter__media-tabs-container,
.block-editor-inserter__block-patterns-tabs-container {
flex-grow: 1;
padding: $grid-unit-20;
height: 100%;
display: flex;
flex-direction: column;
justify-content: space-between;
}

.block-editor-inserter__category-tablist {
display: flex;
flex-direction: column;
border: none;
margin-bottom: $grid-unit-10;
// Push the listitem wrapping the "explore" button to the bottom of the panel.
div[role="listitem"]:last-child {
margin-top: auto;
}

// Temporarily disable the component's indicator animation.
// TODO: remove in favor of using the native component's styles and behavior,
// see https://github.com/WordPress/gutenberg/pull/62879#issuecomment-2219720582
&[aria-orientation="vertical"]::after {
content: none;
}

.block-editor-inserter__category-tab {
// Account for the icon on the right so that it's visually balanced.
padding: $grid-unit-10 $grid-unit-05 $grid-unit-10 $grid-unit-15;
text-align: left;
font-weight: inherit;
display: block;
position: relative;
height: auto;

&[aria-selected="true"] {
color: var(--wp-admin-theme-color);

.components-flex-item {
filter: brightness(0.95);
}

svg {
fill: var(--wp-admin-theme-color);
}
}

&::before {
display: none;
}
}
}

.block-editor-inserter__category-panel {
Expand Down
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
- `RangeControl`: do not tooltip contents to the DOM when not shown ([#65875](https://github.com/WordPress/gutenberg/pull/65875)).
- `Tabs`: fix skipping indication animation glitch ([#65878](https://github.com/WordPress/gutenberg/pull/65878)).

### Enhancements

- `Tabs`: revamped vertical orientation styles ([#65387](https://github.com/WordPress/gutenberg/pull/65387)).

## 28.9.0 (2024-10-03)

### Bug Fixes
Expand Down
152 changes: 109 additions & 43 deletions packages/components/src/tabs/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,15 @@ import * as Ariakit from '@ariakit/react';
*/
import { COLORS, CONFIG } from '../utils';
import { space } from '../utils/space';
import Icon from '../icon';

export const TabListWrapper = styled.div`
position: relative;
display: flex;
align-items: stretch;
flex-direction: row;
text-align: center;
overflow-x: auto;

&[aria-orientation='vertical'] {
flex-direction: column;
text-align: start;
}

:where( [aria-orientation='horizontal'] ) {
Expand All @@ -40,11 +37,12 @@ export const TabListWrapper = styled.div`

@media not ( prefers-reduced-motion ) {
&[data-indicator-animated]::before {
transition-property: transform;
transition-property: transform, border-radius, border-block;
transition-duration: 0.2s;
transition-timing-function: ease-out;
}
}
position: relative;
&::before {
content: '';
position: absolute;
Expand All @@ -59,7 +57,7 @@ export const TabListWrapper = styled.div`
/* Using a large value to avoid antialiasing rounding issues
when scaling in the transform, see: https://stackoverflow.com/a/52159123 */
--antialiasing-factor: 100;
&:not( [aria-orientation='vertical'] ) {
&[aria-orientation='horizontal'] {
--fade-width: 4rem;
--fade-gradient-base: transparent 0%, black var( --fade-width );
--fade-gradient-composed: var( --fade-gradient-base ), black 60%,
Expand Down Expand Up @@ -104,65 +102,87 @@ export const TabListWrapper = styled.div`
${ COLORS.theme.accent };
}
}
&[aria-orientation='vertical']::before {
top: 0;
left: 0;
width: 100%;
height: calc( var( --antialiasing-factor ) * 1px );
transform: translateY( calc( var( --selected-top, 0 ) * 1px ) )
scaleY(
&[aria-orientation='vertical'] {
&::before {
/* Adjusting the border radius to match the scaling in the y axis. */
border-radius: ${ CONFIG.radiusSmall } /
calc(
var( --selected-height, 0 ) / var( --antialiasing-factor )
)
${ CONFIG.radiusSmall } /
(
var( --selected-height, 0 ) /
var( --antialiasing-factor )
)
);
top: 0;
left: 0;
width: 100%;
height: calc( var( --antialiasing-factor ) * 1px );
transform: translateY( calc( var( --selected-top, 0 ) * 1px ) )
scaleY(
calc(
var( --selected-height, 0 ) /
var( --antialiasing-factor )
)
);
background-color: color-mix(
in srgb,
${ COLORS.theme.accent },
transparent 96%
);
background-color: ${ COLORS.theme.gray[ 100 ] };
}
&[data-select-on-move='true']:has(
:is( :focus-visible, [data-focus-visible] )
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably fixed by ariakit/ariakit#4094 which is included in @ariakit/react 0.4.11.

We should follow-up to update ariakit-related dependencies, and consequently we should be able to remove :focus-visible 🤞

)::before {
box-sizing: border-box;
border: var( --wp-admin-border-width-focus ) solid
${ COLORS.theme.accent };
/* Adjusting the border width to match the scaling in the y axis. */
border-block-width: calc(
var( --wp-admin-border-width-focus, 1px ) /
(
var( --selected-height, 0 ) /
var( --antialiasing-factor )
)
);
}
}
`;

export const Tab = styled( Ariakit.Tab )`
& {
scroll-margin: 24px;
flex-grow: 1;
flex-shrink: 0;
display: inline-flex;
align-items: center;
position: relative;
/* Resets */
border-radius: 0;
height: ${ space( 12 ) };
background: transparent;
border: none;
box-shadow: none;

flex: 1 0 auto;
white-space: nowrap;
display: flex;
align-items: center;
cursor: pointer;
line-height: 1.2; // Some languages characters e.g. Japanese may have a native higher line-height.
padding: ${ space( 4 ) };
margin-left: 0;
font-weight: 500;
text-align: inherit;
line-height: 1.2; // Characters in some languages (e.g. Japanese) may have a native higher line-height.
font-weight: 400;
color: ${ COLORS.theme.foreground };

&[aria-disabled='true'] {
cursor: default;
color: ${ COLORS.ui.textDisabled };
}

&:not( [aria-disabled='true'] ):hover {
&:not( [aria-disabled='true'] ):is( :hover, [data-focus-visible] ) {
color: ${ COLORS.theme.accent };
}

&:focus:not( :disabled ) {
position: relative;
box-shadow: none;
outline: none;
}

// Focus.
// Focus indicator.
position: relative;
&::after {
content: '';
position: absolute;
top: ${ space( 3 ) };
right: ${ space( 3 ) };
bottom: ${ space( 3 ) };
left: ${ space( 3 ) };
pointer-events: none;

// Draw the indicator.
Expand All @@ -175,23 +195,69 @@ export const Tab = styled( Ariakit.Tab )`
opacity: 0;

@media not ( prefers-reduced-motion ) {
transition: opacity 0.1s linear;
transition: opacity 0.15s 0.15s linear;
Copy link
Contributor

@t-hamano t-hamano Oct 10, 2024

Choose a reason for hiding this comment

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

Just to be sure, is this intentional? This means the duration is 0.15s and the delay is 0.15s. The Button component has a duration of 0.1s and no delay, which feels a bit unnatural to me:

c7ee84a7135d24ae6cdbc2b1e0360687.mp4

Copy link
Contributor

@ciampo ciampo Oct 10, 2024

Choose a reason for hiding this comment

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

It is: #65387 (comment)

I believe the idea is to show the chevron only after the "indicator" has animated to the new selected tab

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw the comment, but isn't that animation only expected for vertical tabs?

Additionally, I don't know if it's related to this PR, but I can see that the pattern categories in the block sidebar don't seem to animate properly - they don't have a background color when focused:

b98ac34f83e2a4b460f9aff15ddc6803.mp4

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for reporting - I expect @DaniGuardiola to take a look at this when he's back (I don't think he's online today)

Copy link
Contributor

Choose a reason for hiding this comment

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

but isn't that animation only expected for vertical tabs?

This seem like a good point. The delay (and maybe the whole transition) should only be needed for data-select-on-move='true' conditions, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On my phone, and on sick leave, but just wanted to say that as far as I remember, this transition was only applied to the Chevron in vertical tabs, and only when select on move is enabled and when using the arrow keys. If that's not the case, something got messed up, and that'd be surprising to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #65387 (comment) 😅

Will fix soon. Silly me!

Copy link
Contributor

Choose a reason for hiding this comment

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

Fix in #66198 — it should also fix the pattern categories in the block sidebar mentioned by @t-hamano above.

}
}

&:focus-visible::after {
&[data-focus-visible]::after {
opacity: 1;
}
}

[aria-orientation='horizontal'] & {
padding-inline: ${ space( 4 ) };
height: ${ space( 12 ) };
text-align: center;
scroll-margin: 24px;

&::after {
content: '';
inset: ${ space( 3 ) };
}
}

[aria-orientation='vertical'] & {
min-height: ${ space(
10
) }; // Avoid fixed height to allow for long strings that go in multiple lines.
padding: ${ space( 2 ) } ${ space( 3 ) };
min-height: ${ space( 10 ) };
text-align: start;

&[aria-selected='true'] {
color: ${ COLORS.theme.accent };
fill: currentColor;
}
}
[aria-orientation='vertical'][data-select-on-move='false'] &::after {
content: '';
inset: var( --wp-admin-border-width-focus );
}
`;

export const TabChildren = styled.span`
flex-grow: 1;
`;

export const TabChevron = styled( Icon )`
flex-shrink: 0;
margin-inline-end: ${ space( -1 ) };
[aria-orientation='horizontal'] & {
justify-content: center;
display: none;
}
opacity: 0;
[role='tab']:is( [aria-selected='true'], [data-focus-visible], :hover ) & {
opacity: 1;
}
// The chevron is transitioned into existence when selectOnMove is enabled,
// because otherwise it looks jarring, as it shows up outside of the focus
// indicator that's being animated at the same time.
@media not ( prefers-reduced-motion ) {
[data-select-on-move='true']
[role='tab']:is( [aria-selected='true'], )
& {
transition: opacity 0.3s ease-in;
stokesman marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@t-hamano oof, I accidentally replaced the wrong transition, this is where it should have been replaced.

}
}
&:dir( rtl ) {
rotate: 180deg;
}
`;

Expand All @@ -201,7 +267,7 @@ export const TabPanel = styled( Ariakit.TabPanel )`
outline: none;
}

&:focus-visible {
&[data-focus-visible] {
box-shadow: 0 0 0 var( --wp-admin-border-width-focus )
${ COLORS.theme.accent };
// Windows high contrast mode.
Expand Down
10 changes: 8 additions & 2 deletions packages/components/src/tabs/tab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@ import { forwardRef } from '@wordpress/element';
import type { TabProps } from './types';
import warning from '@wordpress/warning';
import { useTabsContext } from './context';
import { Tab as StyledTab } from './styles';
import {
Tab as StyledTab,
TabChildren as StyledTabChildren,
TabChevron as StyledTabChevron,
} from './styles';
import type { WordPressComponentProps } from '../context';
import { chevronRight } from '@wordpress/icons';

export const Tab = forwardRef<
HTMLButtonElement,
Expand All @@ -33,7 +38,8 @@ export const Tab = forwardRef<
render={ render }
{ ...otherProps }
>
{ children }
<StyledTabChildren>{ children }</StyledTabChildren>
<StyledTabChevron icon={ chevronRight } />
</StyledTab>
);
} );
1 change: 1 addition & 0 deletions packages/components/src/tabs/tablist.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ export const TabList = forwardRef<
render={ <TabListWrapper /> }
onBlur={ onBlur }
tabIndex={ -1 }
data-select-on-move={ selectOnMove ? 'true' : 'false' }
{ ...otherProps }
className={ clsx(
overflow.first && 'is-overflowing-first',
Expand Down
7 changes: 7 additions & 0 deletions test/e2e/specs/editor/various/a11y.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,14 @@ test.describe( 'a11y (@firefox, @webkit)', () => {
test( 'should make the modal content focusable when it is scrollable', async ( {
page,
pageUtils,
browserName,
} ) => {
// eslint-disable-next-line playwright/no-skipped-test
test.skip(
browserName === 'webkit',
'Known bug with focus order in Safari.'
);

// Note: this test depends on a particular viewport height to determine whether or not
// the modal content is scrollable. If this tests fails and needs to be debugged locally,
// double-check the viewport height when running locally versus in CI. Additionally,
Expand Down
Loading