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

fix: Snap Section component overflow #29882

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+7 −0
Open

fix: Snap Section component overflow #29882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d7a8315
fix section overflow
hmalik88 Jan 23, 2025
bfbce8e
Merge branch 'main' into hm/fix-text-overflow
hmalik88 Jan 23, 2025
f6e3ddb
update snapshot
hmalik88 Jan 23, 2025
f4c321f
Merge branch 'main' into hm/fix-text-overflow
hmalik88 Jan 23, 2025
941fb9e
Merge branch 'main' into hm/fix-text-overflow
hmalik88 Jan 23, 2025
1daff52
Merge branch 'main' into hm/fix-text-overflow
hmalik88 Feb 7, 2025
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
2 changes: 2 additions & 0 deletions ui/components/app/snaps/snap-ui-renderer/components/box.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
AlignItems,
Display,
FlexDirection,
FlexWrap,
JustifyContent,
TextColor,
} from '../../../../../helpers/constants/design-system';
Expand Down Expand Up @@ -49,5 +50,6 @@ export const box: UIComponentFactory<BoxElement> = ({
alignItems: element.props.center && AlignItems.center,
className: 'snap-ui-renderer__panel',
color: TextColor.textDefault,
flexWrap: FlexWrap.Wrap, // ensures components can wrap horizontally
Copy link
Member

Choose a reason for hiding this comment

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

Both this change and the flex change will have impact beyond the overflow we are trying to fix here.

Have you tested existing Snap UI's to make sure this doesn't cause regressions in behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I tested it with the box being vertical and behavior was unchanged. If you're talking about regressions across all existing snap interfaces (examples and production snaps), no -- that's a lot to check. This and other work has made me think if we should have some service that checks for regressions programmatically, realistically any UI change can break an existing snap interface.

},
});
3 changes: 3 additions & 0 deletions ui/components/app/snaps/snap-ui-renderer/components/text.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ export const text: UIComponentFactory<TextElement> = ({
color: getTextColor(element.props.color),
className: 'snap-ui-renderer__text',
textAlign: element.props.alignment,
style: {
flex: 1, // Ensure text can grow to fill the container
},
},
};
};
2 changes: 2 additions & 0 deletions ...rmations/components/confirm/snaps/snaps-section/__snapshots__/snaps-section.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ exports[`SnapsSection renders section for typed sign request 1`] = `
>
<p
class="mm-box mm-text snap-ui-renderer__text mm-text--body-md mm-text--font-weight-normal mm-text--overflow-wrap-break-word mm-box--color-inherit"
style="flex: 1;"
>
Hello world again!
</p>
Expand Down Expand Up @@ -105,6 +106,7 @@ exports[`SnapsSection renders section personal sign request 1`] = `
>
<p
class="mm-box mm-text snap-ui-renderer__text mm-text--body-md mm-text--font-weight-normal mm-text--overflow-wrap-break-word mm-box--color-inherit"
style="flex: 1;"
>
Hello world!
</p>
Expand Down
Loading