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.

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

chore: updating header to use heading sm #29308

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
+40 −59
Draft

chore: updating header to use heading sm #29308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
35a57a2
chore: updating header to use heading sm
georgewrmarshall Dec 18, 2024
59dd2dc
chore: updating more page headers
georgewrmarshall Dec 29, 2024
13f395b
chore: updating snapshots
georgewrmarshall Dec 29, 2024
8e748b5
chore: permissions heading update
georgewrmarshall Dec 30, 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
8 changes: 0 additions & 8 deletions ui/components/app/assets/nfts/nft-details/__snapshots__/nft-full-image.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,8 @@ exports[`NFT full image should match snapshot 1`] = `
<div
class="mm-box mm-header-base multichain-page-header mm-box--padding-4 mm-box--display-flex mm-box--justify-content-center mm-box--width-full"
>
<div
class="mm-box"
>
<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"
/>
</div>
Comment on lines -17 to -23
Copy link
Contributor Author

@georgewrmarshall georgewrmarshall Dec 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Removes empty p and div tag when children is not present as in the example of the NFT details page header

Screenshot 2024-12-29 at 3 54 07 PM

<div
class="mm-box mm-box--display-flex mm-box--justify-content-flex-end"
style="min-width: 0px;"
>
<button
aria-label="Back"
Expand Down
6 changes: 1 addition & 5 deletions ui/components/app/connected-accounts-permissions/connected-accounts-permissions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,7 @@ const ConnectedAccountsPermissions = ({ permissions }) => {
padding={0}
backgroundColor={BackgroundColor.backgroundDefault}
>
<Text
onClick={toggleExpanded}
as="h6"
variant={TextVariant.bodyMdMedium}
>
<Text onClick={toggleExpanded} variant={TextVariant.bodyMdMedium}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removes incorrect h6 tag

{t('permissions')}
</Text>

Expand Down
8 changes: 4 additions & 4 deletions ui/components/multichain/pages/connections/__snapshots__/connections.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ exports[`Connections Content should render correctly 1`] = `
<div
class="mm-box"
>
<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"
>
<div
class="mm-box connections-header__title mm-box--display-flex mm-box--gap-2 mm-box--justify-content-center mm-box--align-items-center"
Expand All @@ -40,12 +40,12 @@ exports[`Connections Content should render correctly 1`] = `
style="mask-image: url('./images/icons/global.svg');"
/>
<span
class="mm-box mm-text mm-text--heading-md mm-text--ellipsis mm-text--text-align-center mm-box--color-text-default"
class="mm-box mm-text mm-text--heading-sm mm-text--ellipsis mm-text--text-align-center mm-box--color-text-default"
>
metamask.github.io
</span>
</div>
</p>
</h2>
</div>
</div>
<div
Expand Down
2 changes: 1 addition & 1 deletion ui/components/multichain/pages/connections/connections.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ export const Connections = () => {
)}
<Text
as="span"
variant={TextVariant.headingMd}
variant={TextVariant.headingSm}
textAlign={TextAlign.Center}
ellipsis
>
Expand Down
25 changes: 14 additions & 11 deletions ui/components/multichain/pages/page/components/header/header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,20 @@ export const Header = ({
endAccessory={endAccessory}
{...props}
>
<Text
display={Display.Block}
variant={TextVariant.bodyMdBold}
textAlign={TextAlign.Center}
paddingInlineStart={8}
paddingInlineEnd={8}
ellipsis
{...textProps}
>
{children}
</Text>
{children && (
<Text
as="h2"
display={Display.Block}
variant={TextVariant.headingSm}
textAlign={TextAlign.Center}
paddingInlineStart={8}
paddingInlineEnd={8}
ellipsis
{...textProps}
>
{children}
</Text>
)}
Comment on lines +57 to +70
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding logic to only render the h2 tag if children exist see ui/components/app/assets/nfts/nft-details/snapshots/nft-full-image.test.js.snap

</HeaderBase>
);
};
12 changes: 4 additions & 8 deletions ui/components/multichain/pages/permissions-page/__snapshots__/permissions-page.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,11 @@ exports[`All Connections render renders correctly 1`] = `
<div
class="mm-box"
>
<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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating to use heading sm and more correct semantic html

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"
>
<span
class="mm-box mm-text mm-text--heading-md mm-text--text-align-center mm-box--color-text-default"
>
Permissions
</span>
</p>
Permissions
</h2>
</div>
</div>
<div
Expand Down
8 changes: 1 addition & 7 deletions ui/components/multichain/pages/permissions-page/permissions-page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,7 @@ export const PermissionsPage = () => {
/>
}
>
<Text
as="span"
variant={TextVariant.headingMd}
textAlign={TextAlign.Center}
>
{t('permissions')}
</Text>
Comment on lines -80 to -86
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing extra html element to reduce html bloat and to maintain heading consistency

{t('permissions')}
</Header>
<Content padding={0}>
<Box ref={headerRef}></Box>
Expand Down
8 changes: 4 additions & 4 deletions ...tichain/pages/review-permissions-page/__snapshots__/review-permissions-page.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ exports[`ReviewPermissions should render correctly 1`] = `
<div
class="mm-box"
>
<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"
>
<div
class="mm-box connections-header__title mm-box--display-flex mm-box--gap-2 mm-box--justify-content-center mm-box--align-items-center"
Expand All @@ -40,10 +40,10 @@ exports[`ReviewPermissions should render correctly 1`] = `
style="mask-image: url('./images/icons/global.svg');"
/>
<span
class="mm-box mm-text mm-text--heading-md mm-text--ellipsis mm-text--text-align-center mm-box--color-text-default"
class="mm-box mm-text mm-text--heading-sm mm-text--ellipsis mm-text--text-align-center mm-box--color-text-default"
/>
</div>
</p>
</h2>
</div>
</div>
<div
Expand Down
4 changes: 2 additions & 2 deletions ui/components/multichain/pages/send/__snapshots__/send.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ exports[`SendPage render and initialization should render correctly even when a
<div
class="mm-box"
>
<h4
<h2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating header tag

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"
>
Send
</h4>
</h2>
</div>
</div>
<div
Expand Down
2 changes: 1 addition & 1 deletion ui/components/multichain/permissions-header/permissions-header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const PermissionsHeader = ({
)}
<Text
as="span"
variant={TextVariant.headingMd}
variant={TextVariant.headingSm}
textAlign={TextAlign.Center}
ellipsis
>
Expand Down
4 changes: 2 additions & 2 deletions ui/pages/bridge/__snapshots__/index.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ exports[`Bridge renders the component with initial props 1`] = `
<div
class="mm-box"
>
<h4
<h2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating header tag

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"
>
Bridge
</h4>
</h2>
</div>
<div
class="mm-box mm-box--display-flex mm-box--justify-content-flex-end"
Expand Down
12 changes: 6 additions & 6 deletions ui/pages/permissions-connect/connect-page/__snapshots__/connect-page.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ exports[`ConnectPage should render correctly 1`] = `
<div
class="mm-box"
>
<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"
>
<h2
class="mm-box mm-text mm-text--heading-lg mm-box--color-text-default"
Expand All @@ -29,7 +29,7 @@ exports[`ConnectPage should render correctly 1`] = `
This site wants to
:
</p>
</p>
</h2>
</div>
</div>
<div
Expand Down Expand Up @@ -265,8 +265,8 @@ exports[`ConnectPage should render with defaults from the requested permissions
<div
class="mm-box"
>
<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"
>
<h2
class="mm-box mm-text mm-text--heading-lg mm-box--color-text-default"
Expand All @@ -279,7 +279,7 @@ exports[`ConnectPage should render with defaults from the requested permissions
This site wants to
:
</p>
</p>
</h2>
</div>
</div>
<div
Expand Down
Loading