fix(card): added text wrapping to cta text #7451

BrunnoM7 · 2021-10-20T11:59:30Z

Related Ticket(s)

[Card] CTA text doesn't wrap #6935

Description

Corrects the over extension of the CTA text in the Card by wrapping it.

Changelog

New

Added the boolean property iconInline to card-footer component
If the iconPlacement is set a right and iconInline is true text CTA will have the wrap behavior.

…on-for-ibm-dotcom into fix/card-cta-text-wrap

ibmdotcom-bot · 2021-10-20T12:25:35Z

Deploy preview created for package React:
https://ibmdotcom-react.s3.us-south.cloud-object-storage.appdomain.cloud/deploy-previews/7451/index.html

Built with commit: 15d0a5255d92cad2966d09fef91ae9a6da92e0af

ibmdotcom-bot · 2021-10-20T12:32:48Z

Deploy preview created for package Web Components HTML Test Application:
https://ibmdotcom-web-components-html-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/7451/index.html

Built with commit: 15d0a5255d92cad2966d09fef91ae9a6da92e0af

ibmdotcom-bot · 2021-10-20T12:32:55Z

Deploy preview created for package Web Components:
https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/7451/index.html

Built with commit: 15d0a5255d92cad2966d09fef91ae9a6da92e0af

ibmdotcom-bot · 2021-10-20T12:34:29Z

Deploy preview created for package Web Components (React wrapper):
https://ibmdotcom-web-components-react.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/7451/index.html

Built with commit: 15d0a5255d92cad2966d09fef91ae9a6da92e0af

ibmdotcom-bot · 2021-10-20T13:11:41Z

Deploy preview created for package NextJS Test Application:
https://ibmdotcom-nextjs-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/7451/index.html

Built with commit: 15d0a5255d92cad2966d09fef91ae9a6da92e0af

ibmdotcom-bot · 2021-10-20T13:17:33Z

Deploy preview created for package "Web Components (Codesandbox Examples)":
https://webcomponents-codesandbox.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/7451/index.html

Built with commit: 15d0a5255d92cad2966d09fef91ae9a6da92e0af

ibmdotcom-bot · 2021-10-20T13:41:41Z

Deploy preview created for package "React (Codesandbox Examples)":
https://react-codesandbox.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/7451/index.html

Built with commit: 15d0a5255d92cad2966d09fef91ae9a6da92e0af

emyarod

we should be able to continue using flexbox to resolve the root issue so changing the footer container to a block element may not be needed at all!

emyarod · 2021-10-20T13:54:55Z

packages/web-components/src/components/card-group/card-group.ts

@@ -127,9 +127,7 @@ class DDSCardGroup extends StableSelectorMixin(LitElement) {
      const { customPropertyCardsPerRow } = this.constructor as typeof DDSCardGroup;


are these changes intentional or are they carried over from a separate branch maybe?

I believe this was carried over since I didn't touch this file.

emyarod · 2021-10-20T13:56:42Z

packages/styles/scss/components/card/index.scss

+      display: block;

      svg,
      ::slotted(svg[slot='icon']) {
-        display: block;
+        display: inline;
+        transform: translate(-4px, 6px);
      }


instead of changing the display rule from flex to block I think we can just set a flex-shrink: 1 on bx--card__cta__copy to resolve the original issue

I tried to use flex-shrink: 1 on bx--card__cta__copy, it wraps the text but the svg doesn't behave as part of the text as expected. It ends up behaving like this:

oh I see, would we be able to use the iconInline property that is on link-with-icon ? #6812

Thanks for showing me this PR. I applied a similar solution as is on link-with-icon, had to add a selector to the style sheet in order to get the card-footer, but it should be working fine now. Also reverted the previews changes to the CSS.

jeffchew · 2021-10-23T14:26:20Z

@BrunnoM7 can you provide more details in your PR review description on what this is trying to solve, and what has changed?

Link(s) would also help as far as what storybook links the reviewers should be looking at.

shixiedesign · 2021-10-25T13:57:25Z

Almost there! Small change:
There should be a padding right of 32px on desktop. No padding on sm:

BrunnoM7 · 2021-10-25T18:46:48Z

Hey @shixiedesign, thanks for the review. I just added the right padding to the CTA text. It should be working fine now.

shixiedesign · 2021-10-28T13:12:01Z

Trying to review this, but need to update branch to get some updated Percy tests. Will return later today.

shixiedesign · 2021-10-28T13:13:39Z

@BrunnoM7 is this fix not working in React?

…on-for-ibm-dotcom into pr/7451

…tion" This reverts commit 02d3279.

…on-for-ibm-dotcom into pr/7451

shixiedesign

Looks good! Thank you

emyarod

looks good to me!

github-actions · 2021-11-05T20:22:54Z

DCO Assistant Lite bot All Contributors have signed the CLA.

jeffchew · 2021-11-08T13:21:21Z

Hey @BrunnoM7 , once you sign the DCO this can get merged in.

BrunnoM7 · 2021-11-08T13:26:48Z

I have read the DCO document and I hereby sign the DCO.

BrunnoM7 added 3 commits October 15, 2021 19:36

chore(card-group): fixed card border-top and grid mode transition

02d3279

Merge branch 'master' of https://github.com/carbon-design-system/carb…

0ac183b

…on-for-ibm-dotcom into fix/card-cta-text-wrap

fix(card): cta text wrap

93d9716

BrunnoM7 requested a review from a team as a code owner October 20, 2021 11:59

BrunnoM7 requested review from kennylam, emyarod and oliviaflory October 20, 2021 11:59

jeffchew changed the title ~~Fix(Card): added text wrapping to cta text~~ fix(card): added text wrapping to cta text Oct 20, 2021

jeffchew added the 👀 eyes needed label Oct 20, 2021

emyarod reviewed Oct 20, 2021

View reviewed changes

emyarod added 🛠️ fix needed and removed 👀 eyes needed labels Oct 20, 2021

shixiedesign added the Needs design approval PRs on feature requests and new components have to get design approval before merge. label Oct 22, 2021

shixiedesign self-assigned this Oct 22, 2021

fix(card): revert scss applied solution via props

241d126

BrunnoM7 added 👀 eyes needed and removed 🛠️ fix needed labels Oct 22, 2021

emyarod mentioned this pull request Oct 22, 2021

[PR Reviews]: Andrew - Sprint 21 #7491

Closed

shixiedesign added 🛠️ fix needed and removed 👀 eyes needed labels Oct 25, 2021

fix(card): fixed padding-right on CTA text

74f9958

BrunnoM7 added 👀 eyes needed and removed 🛠️ fix needed labels Oct 25, 2021

Merge branch 'master' into fix/card-cta-text-wrap

237e451

shixiedesign added 🛠️ fix needed and removed 👀 eyes needed labels Oct 30, 2021

BrunnoM7 added 5 commits November 3, 2021 17:54

Merge branch 'master' of https://github.com/carbon-design-system/carb…

d4ec50a

…on-for-ibm-dotcom into pr/7451

fix(card): revert changes to card feature

0663a64

Revert "chore(card-group): fixed card border-top and grid mode transi…

dba7948

…tion" This reverts commit 02d3279.

Merge branch 'master' of https://github.com/carbon-design-system/carb…

1119ee2

…on-for-ibm-dotcom into pr/7451

fix(card): fixed icon placement on react feature card

eabafba

BrunnoM7 added 👀 eyes needed and removed 🛠️ fix needed labels Nov 4, 2021

shixiedesign removed the Needs design approval PRs on feature requests and new components have to get design approval before merge. label Nov 4, 2021

shixiedesign approved these changes Nov 4, 2021

View reviewed changes

Merge branch 'master' into fix/card-cta-text-wrap

7a36262

emyarod approved these changes Nov 5, 2021

View reviewed changes

jeffchew added Ready to merge Label for the pull requests that are ready to merge and removed 👀 eyes needed labels Nov 5, 2021

Merge branch 'master' into fix/card-cta-text-wrap

1079d57

This comment has been minimized.

Sign in to view

kodiakhq bot added 2 commits November 5, 2021 19:23

Merge branch 'master' into fix/card-cta-text-wrap

9f91ad4

Merge branch 'master' into fix/card-cta-text-wrap

fc915d4

Merge branch 'master' into fix/card-cta-text-wrap

15d0a52

kodiakhq bot merged commit 5cf7ef3 into carbon-design-system:master Nov 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(card): added text wrapping to cta text #7451

fix(card): added text wrapping to cta text #7451

BrunnoM7 commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

emyarod left a comment

emyarod Oct 20, 2021

BrunnoM7 Oct 20, 2021

emyarod Oct 20, 2021

BrunnoM7 Oct 20, 2021

emyarod Oct 20, 2021

BrunnoM7 Oct 22, 2021

jeffchew commented Oct 23, 2021 •

edited

Loading

shixiedesign commented Oct 25, 2021

BrunnoM7 commented Oct 25, 2021

shixiedesign commented Oct 28, 2021 •

edited

Loading

shixiedesign commented Oct 28, 2021

shixiedesign left a comment

emyarod left a comment

This comment has been minimized.

github-actions bot commented Nov 5, 2021 •

edited

Loading

jeffchew commented Nov 8, 2021

BrunnoM7 commented Nov 8, 2021

		@@ -127,9 +127,7 @@ class DDSCardGroup extends StableSelectorMixin(LitElement) {
		const { customPropertyCardsPerRow } = this.constructor as typeof DDSCardGroup;

fix(card): added text wrapping to cta text #7451

fix(card): added text wrapping to cta text #7451

Conversation

BrunnoM7 commented Oct 20, 2021 • edited Loading

Related Ticket(s)

Description

Changelog

ibmdotcom-bot commented Oct 20, 2021 • edited Loading

ibmdotcom-bot commented Oct 20, 2021 • edited Loading

ibmdotcom-bot commented Oct 20, 2021 • edited Loading

ibmdotcom-bot commented Oct 20, 2021 • edited Loading

ibmdotcom-bot commented Oct 20, 2021 • edited Loading

ibmdotcom-bot commented Oct 20, 2021 • edited Loading

ibmdotcom-bot commented Oct 20, 2021 • edited Loading

emyarod left a comment

Choose a reason for hiding this comment

emyarod Oct 20, 2021

Choose a reason for hiding this comment

BrunnoM7 Oct 20, 2021

Choose a reason for hiding this comment

emyarod Oct 20, 2021

Choose a reason for hiding this comment

BrunnoM7 Oct 20, 2021

Choose a reason for hiding this comment

emyarod Oct 20, 2021

Choose a reason for hiding this comment

BrunnoM7 Oct 22, 2021

Choose a reason for hiding this comment

jeffchew commented Oct 23, 2021 • edited Loading

shixiedesign commented Oct 25, 2021

BrunnoM7 commented Oct 25, 2021

shixiedesign commented Oct 28, 2021 • edited Loading

shixiedesign commented Oct 28, 2021

shixiedesign left a comment

Choose a reason for hiding this comment

emyarod left a comment

Choose a reason for hiding this comment

This comment has been minimized.

github-actions bot commented Nov 5, 2021 • edited Loading

jeffchew commented Nov 8, 2021

BrunnoM7 commented Nov 8, 2021

BrunnoM7 commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

ibmdotcom-bot commented Oct 20, 2021 •

edited

Loading

jeffchew commented Oct 23, 2021 •

edited

Loading

shixiedesign commented Oct 28, 2021 •

edited

Loading

github-actions bot commented Nov 5, 2021 •

edited

Loading