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

[Design Quality]: Update segmented tab styling #26190

Closed
amandaye0h opened this issue Jul 29, 2024 · 5 comments
Closed

[Design Quality]: Update segmented tab styling #26190

amandaye0h opened this issue Jul 29, 2024 · 5 comments
Assignees
Labels
area-design Design bug (previously known as papercuts - ask Hilary for more detail) external-contributor INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. regression-main Regression bug that was found on main branch, but not yet present in production Sev3-low Low severity; minimal to no impact upon users team-wallet-ux type-enhancement

Comments

@amandaye0h
Copy link

amandaye0h commented Jul 29, 2024

Problem statement

Visually, our tabs look unbalanced and compete for attention with other components. The updated styling strengthens the contrast between primary and secondary elements to help users focus stay focused on key tasks.

Acceptance criteria

  • Inactive tab text color is updated from text-default to text-muted
  • Border styling for inactive tab =: 2px solid color: var(--color-text-muted)
  • 4px padding between tabs has been removed
  • font-weight = 500 (medium) for active and inactive tabs

Testing

A similar styling is applied in other flows:

  • Send Flow (Your accounts/Contacts)
  • Asset picker modal (Tokens/NFTs)
  • Permissions page (Sites/Snaps)
  • dApp connections (Connected accounts/Connected snaps)
  • Import tokens (Search/Custom)

Final review

Key screen

Figma link
Screenshot 2024-07-29 at 7 38 10 AM

Related screens

Figma link
Screenshot 2024-07-30 at 8 49 36 AM

Steps to reproduce

Portfolio View

  1. Open MetaMask

Import tokens

  1. Open MetaMask
  2. Scroll down
  3. Click on import tokens

Send Flow

  1. Open MetaMask
  2. Click on Send button
  3. Click on the Asset selector (ETH) to access the modal

Permissions
(Associated with Amon/Hen)

  1. Open MetaMask
  2. Click on three dot menu in the top-right of the header
  3. Click on Permissions

dApp connection page

  1. Open MetaMask
  2. Click on dApp icon in the header

Error messages or log output

No response

Detection stage

On the development branch

Version

11.16.15

Build type

None

Browser

Brave

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

Severity

No response

@amandaye0h amandaye0h added the type-bug Something isn't working label Jul 29, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Jul 29, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Jul 29, 2024
@metamaskbot metamaskbot added external-contributor regression-main Regression bug that was found on main branch, but not yet present in production INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. labels Jul 29, 2024
@hilvmason hilvmason added the area-design Design bug (previously known as papercuts - ask Hilary for more detail) label Jul 30, 2024
@desi desi added team-wallet-ux Sev3-low Low severity; minimal to no impact upon users labels Aug 12, 2024
@hilvmason hilvmason added type-enhancement and removed type-bug Something isn't working labels Aug 15, 2024
@matteoscurati matteoscurati self-assigned this Jan 13, 2025
@matteoscurati matteoscurati moved this from To be fixed to Fix in Progress in Bugs by team Jan 13, 2025
@matteoscurati
Copy link
Contributor

@amandaye0h I’m creating a PR to fix the tab styles as indicated here. I have two questions:

  1. how should we handle cases where a tab list has only one tab? For now, I’ve created a style that applies text-default and border none (see image). Is this correct?
  2. I believe some examples linked in Figma are outdated. For instance, the Snap section now has its own dedicated page and no longer lives under Permissions, correct?
Image

@amandaye0h
Copy link
Author

amandaye0h commented Jan 13, 2025

Thanks for working on this!

  1. In the case above, the tab should not exist. This is the intended UI:
Image
  1. Yes that is correct. My bad for including the outdated designs!

@matteoscurati
Copy link
Contributor

hey @amandaye0h thanks for your response :) I think I have resolved this issue in this PR #26190 but I’m still convinced that an edge case remains where a Tab group has only one tab. Do you think the style I’ve proposed could work in this case? Or do you think the single tab should still be highlighted as a tab (with blue font and a blue bottom border)?

@matteoscurati
Copy link
Contributor

This is the PR with a proposed solution to standardize the tab style #29652

@amandaye0h
Copy link
Author

Yeah, I think it's an acceptable solution for now — best case scenario is that users don't see the tab at all.

I'm ok with the white text if it's too complex to remove the tab altogether. The blue text + blue border is visually way too loud.

@github-project-automation github-project-automation bot moved this from Fix in Progress to Fixed in Bugs by team Jan 18, 2025
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by severity Jan 18, 2025
github-merge-queue bot pushed a commit that referenced this issue Jan 30, 2025
## **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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-design Design bug (previously known as papercuts - ask Hilary for more detail) external-contributor INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. regression-main Regression bug that was found on main branch, but not yet present in production Sev3-low Low severity; minimal to no impact upon users team-wallet-ux type-enhancement
Projects
Archived in project
Status: Fixed
Development

No branches or pull requests

5 participants