Skip to content

Item: Unify focus style and add default font styles #52495

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

Merged
t-hamano merged 10 commits into from
Jul 13, 2023
Merged

Item: Unify focus style and add default font styles #52495

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
48a9691
ItemGroup: Apply focus state to `Item` component rendered as anchor e…
t-hamano Jul 11, 2023
ebe07be
Update changelog
t-hamano Jul 11, 2023
821a377
Don't apply text-decoration in the case of a button element
t-hamano Jul 11, 2023
1194265
Update snapshot
t-hamano Jul 11, 2023
5e88060
Merge branch 'trunk' into item/fix-focus-state-as-a
t-hamano Jul 12, 2023
48b6050
Update storybook and add default styles to anchor element
t-hamano Jul 12, 2023
672e399
Update snapshot
t-hamano Jul 12, 2023
74cdb6b
Update changelog
t-hamano Jul 12, 2023
9e29aab
Resolve conflicts
t-hamano Jul 13, 2023
51ef2f8
Fix incorrect changelog entries
t-hamano Jul 13, 2023
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
Next Next commit
ItemGroup: Apply focus state to Item component rendered as anchor e… 
…lement
  • Loading branch information
@t-hamano
t-hamano committed Jul 11, 2023
commit 48a9691a913350f8c925cd2434c844c8aa08668f
2 changes: 1 addition & 1 deletion packages/components/src/item-group/item/hook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function useItem( props: WordPressComponentProps< ItemProps, 'div' > ) {
const classes = useMemo(
() =>
cx(
as === 'button' && styles.unstyledButton,
( as === 'button' || as === 'a' ) && styles.unstyledButton,
styles.itemSizes[ size ] || styles.itemSizes.medium,
styles.item,
spacedAround && styles.spacedAround,
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/item-group/styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export const unstyledButton = css`
cursor: pointer;
background: none;
text-align: start;
text-decoration: none;

svg,
path {
Expand Down
11 changes: 0 additions & 11 deletions packages/edit-site/src/components/sidebar-navigation-item/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,6 @@
fill: $gray-700;
}

&:is(a) {
text-decoration: none;
display: flex;
align-items: center;

&:focus {
box-shadow: none;
outline: none;
}
Comment on lines -34 to -37
Copy link
Member

@mirka mirka Jul 12, 2023

Choose a reason for hiding this comment

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

It looks like we still need to keep this reset, because the :focus state gets triggered (without being overridden by a :focus-visible state) when you click on the link:

The anchor item gets a light blue box-shadow when clicked

Copy link
Contributor Author

@t-hamano t-hamano Jul 12, 2023

Choose a reason for hiding this comment

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

This focus style seems to be derived from WP-Admin's common.css.

common

I think it would be better to resolve this at the component level, so I will move this focus style to the component.

}

&.with-suffix {
padding-right: $grid-unit-20;
}
Expand Down