Skip to content

Commit

Permalink
fix: update segmented tab styling (#29652)
Browse files Browse the repository at this point in the history
## **Description**

This PR updates the style of the
`ui/components/ui/tabs/tabs.component.js` component and removes some
custom classes used to override the style in various parts of the site
(particularly on the home page and the notifications page). This ensures
a consistent style for the tabs.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29652?quickstart=1)

## **Related issues**

Original request:
#26190

## **Manual testing steps**

The tabs tested are those indicated in this issue:
#28910. However,
some of them seem to no longer be in use (for example, the “snap” tab in
the permissions page should no longer exist, as it now has its own
dedicated page).

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

### **After**

<img width="453" alt="Screenshot 2025-01-13 at 09 49 49"
src="https://github.com/user-attachments/assets/55aeaf12-fa5e-4c8c-8f4b-4ab9bfab29ff"
/>
<img width="366" alt="Screenshot 2025-01-12 at 23 28 37"
src="https://github.com/user-attachments/assets/935de0dc-e0dd-4439-8ec0-754ded9a256b"
/>
<img width="358" alt="Screenshot 2025-01-12 at 23 28 23"
src="https://github.com/user-attachments/assets/fa1cb8b8-97f4-483b-aedc-afdc91d9fb33"
/>
<img width="361" alt="Screenshot 2025-01-12 at 23 28 08"
src="https://github.com/user-attachments/assets/8d9b2414-57f9-48ba-99ec-80e48efe1379"
/>

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
matteoscurati authored Jan 30, 2025
1 parent 267be65 commit 4e1b0dc
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 12 deletions.
2 changes: 0 additions & 2 deletions ui/components/multichain/account-overview/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
@include design-system.H6;

flex-grow: 1;
color: var(--color-icon-default);
font-weight: 500;
}

&__tab--active {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ exports[`SendPage render and initialization should render correctly even when a
class="box tabs box--flex-direction-row"
>
<ul
class="box tabs__list box--display-flex box--gap-1 box--flex-direction-row box--justify-content-flex-start box--background-color-background-default"
class="box tabs__list box--display-flex box--flex-direction-row box--justify-content-flex-start box--background-color-background-default"
>
<li
class="box tab tab--active box--flex-direction-row"
Expand Down
1 change: 0 additions & 1 deletion ui/components/ui/tabs/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
-moz-transform: translateZ(0);

&__list {
border-bottom: 1px solid var(--color-border-default);
position: sticky;
top: 0;
z-index: 2;
Expand Down
10 changes: 10 additions & 0 deletions ui/components/ui/tabs/tab/index.scss
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
.tab {
border-bottom: 2px solid var(--color-border-muted);
color: var(--color-text-muted);
font-weight: 500;

button {
min-width: 50px;
background-color: unset;
color: unset;
font-weight: 500;
}

&--single {
color: var(--color-text-default);
border-bottom: none;
}

&--active {
Expand Down
3 changes: 3 additions & 0 deletions ui/components/ui/tabs/tab/tab.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const Tab = (props) => {
className,
'data-testid': dataTestId,
isActive,
isSingleTab,
name,
onClick,
tabIndex,
Expand All @@ -32,6 +33,7 @@ const Tab = (props) => {
<Box
as="li"
className={classnames('tab', className, {
'tab--single': isSingleTab,
'tab--active': isActive,
[activeClassName]: activeClassName && isActive,
})}
Expand Down Expand Up @@ -64,6 +66,7 @@ Tab.propTypes = {
className: PropTypes.string,
'data-testid': PropTypes.string,
isActive: PropTypes.bool, // required, but added using React.cloneElement
isSingleTab: PropTypes.bool, // required, but added using React.cloneElement
name: PropTypes.node.isRequired,
tabKey: PropTypes.string.isRequired, // for Tabs selection purpose
onClick: PropTypes.func,
Expand Down
4 changes: 3 additions & 1 deletion ui/components/ui/tabs/tabs.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,14 @@ const Tabs = ({

return React.Children.map(_getValidChildren(), (child, index) => {
const tabKey = child?.props.tabKey;
const isSingleTab = numberOfTabs === 1;
return (
child &&
React.cloneElement(child, {
onClick: (idx) => handleTabClick(idx, tabKey),
tabIndex: index,
isActive: numberOfTabs > 1 && index === activeTabIndex,
isSingleTab,
})
);
});
Expand Down Expand Up @@ -81,7 +83,7 @@ const Tabs = ({
justifyContent={JustifyContent.flexStart}
backgroundColor={BackgroundColor.backgroundDefault}
className={classnames('tabs__list', tabsClassName)}
gap={1}
gap={0}
>
{renderTabs()}
</Box>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ exports[`Confirm Transaction Base should match snapshot 1`] = `
class="box tabs box--flex-direction-row"
>
<ul
class="box tabs__list box--display-flex box--gap-1 box--flex-direction-row box--justify-content-flex-start box--background-color-background-default"
class="box tabs__list box--display-flex box--flex-direction-row box--justify-content-flex-start box--background-color-background-default"
>
<li
class="box tab confirm-page-container-content__tab tab--active box--flex-direction-row"
Expand Down
6 changes: 0 additions & 6 deletions ui/pages/notifications/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,6 @@
@include design-system.H6;

flex-grow: 1;
color: var(--color-icon-default);
font-weight: 500;
}

&__tab--active {
color: var(--color-primary-default);
}

&__tab button {
Expand Down

0 comments on commit 4e1b0dc

Please sign in to comment.