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

chore: updating header to use heading sm #29308

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

georgewrmarshall
Copy link
Contributor

@georgewrmarshall georgewrmarshall commented Dec 18, 2024

Description

This PR updates the header component text styling to use semantic HTML and consistent heading styles. The changes include:

  • Changed text element from <p> to <h2>
  • Updated text variant from bodyMdBold to headingSm
  • Improves accessibility by using proper heading hierarchy
  • Maintains consistent styling across multiple pages using the header component

Open in GitHub Codespaces

Related issues

Partially fixes: #29306

Manual testing steps

  1. Navigate to pages using the header component (NFT details, Connections, Permissions pages)
  2. Verify header text appears correctly with new heading styles
  3. Verify layout and alignment remain consistent
  4. Check that accessibility tools properly recognize the heading structure

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • 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.

@georgewrmarshall georgewrmarshall added the team-design-system All issues relating to design system in Extension label Dec 18, 2024
@georgewrmarshall georgewrmarshall self-assigned this Dec 18, 2024
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

Comment on lines -20 to 22
<p
class="mm-box mm-text mm-text--body-md-bold mm-text--ellipsis mm-text--text-align-center mm-box--padding-inline-start-8 mm-box--padding-inline-end-8 mm-box--display-block mm-box--color-text-default"
<h2
class="mm-box mm-text mm-text--heading-sm mm-text--ellipsis mm-text--text-align-center mm-box--padding-inline-start-8 mm-box--padding-inline-end-8 mm-box--display-block mm-box--color-text-default"
/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add logic to remove the Text component if children is not present

@metamaskbot
Copy link
Collaborator

Builds ready [006e636]
Page Load Metrics (1543 ± 90 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint13882147154518890
domContentLoaded13652093151618287
load13882147154318790
domInteractive217636178
backgroundConnect9140283014
firstReactRender1596473115
getState4100212512
initialActions01000
loadScripts9681614112915474
setupStore611721
uiStartup157226771814301144
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 7 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-design-system All issues relating to design system in Extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Standardize headers across the extension
2 participants