Fix Banner styling for custom action buttons #2260
Conversation
… as a custom action in the banner component. This is fixed by wrapping the custom action with flex-shrink: 0 so that the custom action can overflow. The set height is not removed so that the alignment can still stay the same
🦋 Changeset detectedLatest commit: ec591bd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Size Change: +1 B (0%) Total Size: 97.1 kB
ℹ️ View Unchanged
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-ygxcaiagiz.chromatic.com/ Chromatic results:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2260 +/- ##
==========================================
+ Coverage 94.70% 95.76% +1.06%
==========================================
Files 248 248
Lines 29513 29514 +1
Branches 1668 2417 +749
==========================================
+ Hits 27950 28265 +315
+ Misses 1559 1241 -318
- Partials 4 8 +4
... and 41 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (ea52fc7) and published all packages with changesets to npm. You can install the packages in webapp by running: ./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2260"Packages can also be installed manually by running: yarn add @khanacademy/wonder-blocks-<package-name>@PR2260 |
| @@ -204,7 +204,7 @@ const Banner = (props: Props): React.ReactElement => { | |||
| if (action.type === "custom") { | |||
| return ( | |||
| <View style={styles.action} key={`custom-action-${i}`}> | |||
| {action.node} | |||
| <View style={styles.customAction}>{action.node}</View> | |||
There was a problem hiding this comment.
question: Would it be possible to merge this style in the parent View? I'm not sure if that will affect applying flexShrink in there? if it can be done, it would be better as we avoid introducing more divs in that component.
<View style={[styles.action, styles.customAction]} key...>{action.node}</div>
There was a problem hiding this comment.
Great question! flexShrink needs to be applied to the child of the flex container. It would be great if we could use a child combinator (> *) in styles.action, but I wasn't able to get that working! Let me know if there is another way we could apply the styles to the children element(s)!
There was a problem hiding this comment.
I found another solution that doesn't require adding an extra div! Thanks for the suggestion of moving the height to the actions container 😄
Summary:
Fixing styling issue where non-tertiary buttons get cropped when used as a custom action in the banner component.
Issue: WB-1716
Test plan:
?path=/story/packages-banner--with-custom-action-primary)?path=/story/packages-banner--with-mixed-actions)Screenshots:
Mixed actions (before & after changes)
Primary button for custom action (before & after changes)
