[SDK-431] notification-buttons-overflow-with-large-text#859
[SDK-431] notification-buttons-overflow-with-large-text#859lposen wants to merge 2 commits intoemb-ootb/masterfrom
Conversation
…ility and visual consistency
|
|
Coverage Impact This PR will not change total coverage. 🚦 See full report on Qlty Cloud »🛟 Help
|
| @@ -88,7 +88,6 @@ export const Embedded = () => { | |||
|
|
|||
| return ( | |||
| <SafeAreaView style={styles.container}> | |||
There was a problem hiding this comment.
The changes that I made to this file is just so that the example page is not so crowded
2 new issues
|
There was a problem hiding this comment.
Pull request overview
Addresses SDK-431 by preventing long CTA button text in Embedded Notification views from overflowing its container, improving layout resilience for localized/large button titles.
Changes:
- Truncate Notification button titles to a single line with tail ellipsis.
- Update Notification button/container flex styles to allow shrinking and wrapping within the container.
- Adjust related styling/example UI (Banner button font weight; example Embedded screen layout tweaks).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/embedded/components/IterableEmbeddedNotification/IterableEmbeddedNotification.tsx | Adds numberOfLines + ellipsizeMode to prevent long CTA text overflow. |
| src/embedded/components/IterableEmbeddedNotification/IterableEmbeddedNotification.styles.ts | Updates flex/wrapping/shrinking styles for buttons and button text. |
| src/embedded/components/IterableEmbeddedBanner/IterableEmbeddedBanner.styles.ts | Changes banner button text weight (unrelated to stated PR scope). |
| example/src/components/Embedded/Embedded.tsx | Reworks example screen controls/layout. |
| example/src/components/Embedded/Embedded.styles.ts | Adds new example styles; minor formatting inconsistency introduced. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <Text | ||
| ellipsizeMode="tail" | ||
| numberOfLines={1} | ||
| style={[ |
There was a problem hiding this comment.
New truncation behavior (numberOfLines/ellipsizeMode) isn’t covered by tests. Since this repo already has @testing-library tests for IterableEmbeddedNotification, add an assertion that the button title renders with numberOfLines={1} (and ellipsizeMode="tail") so this overflow fix doesn’t regress.
| buttonText: { | ||
| fontSize: 14, | ||
| fontWeight: '400', | ||
| fontWeight: '700', | ||
| lineHeight: 20, |
There was a problem hiding this comment.
This PR is scoped to fixing Notification button overflow, but this change also alters Banner button typography (fontWeight 400 → 700). If this is intended, please mention it in the PR description / ticket context; otherwise consider reverting or splitting into a separate PR to avoid unexpected visual changes for Banner consumers.
| container:{ | ||
| ...container, | ||
| marginHorizontal: 0, | ||
| marginTop: 0, | ||
| paddingHorizontal: 0, | ||
| }, |
There was a problem hiding this comment.
Minor formatting inconsistency: container:{ is missing a space after the colon. Other style objects in the example app use key: { (e.g., Commerce.styles.ts). Update to container: { to keep formatting consistent and avoid churn in future diffs.
🔹 JIRA Ticket(s) if any
✏️ Description
Fix long buttons overflowing container
