[Component] Text introduction #193

meissadia · 2023-10-05T17:25:02Z

Closes #158
Closes #157
Closes #162

Sorry for the scope creep here....

Changes

Creates Paragraph component (+ tests + stories) used in TextIntroductions
- Verified that lead-paragraph reflects the removal of top margin via the DS update
Creates TextIntroductions component (+ tests + stories) using new DS component
Updates Storybook theme
- Removes light/dark theme (this was added as a nice-to-have, was never requested)
- Refactors theme implementation and colors
Sets Storybook UI to use line-height: 22px without interfering with the component story styles (ex. Lead paragraph displays with line-height: 27.5px)

How to test this PR

yarn vitest TextIntro
yarn vitest Paragraph

Screenshots

Component	Sidebar	Overview
TextIntroduction
Paragraph

netlify · 2023-10-05T17:25:07Z

✅ Deploy Preview for cfpb-design-system-react ready!

Name	Link
🔨 Latest commit	`da513ce`
🔍 Latest deploy log	https://app.netlify.com/sites/cfpb-design-system-react/deploys/65413793a9695d0008fc0774
😎 Deploy Preview	https://deploy-preview-193--cfpb-design-system-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

natalia-fitzgerald

@meissadia
I still need to review the Text introduction as a component. The review below is for the Lead paragraph and Paragraph.

Lead paragraph

Change placeholder for lead paragraph to:
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.

Paragraph

Change placeholder for paragraph to:
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore.

Global Storybook style:

For paragraph text all Storybook pages (global) correct the line height. It is currently set to 24px. It should be 22px to match the paragraph styling.
Change text color to CFPB Black (#101820) - It is currently set to Gray - #2E3438

Is this the current spacing at desktop?

<h1> top margin: 0px; bottom-margin: 15px
<div class="lead-paragraph"> top-margin: 30px; bottom-margin: 18.33px
<p> top margin: 0px; bottom-margin: 15px
Margin between H1 and Lead Paragraph: 30-px; Lead paragraph and Paragraph: 18.33px; Paragraph and Link: 15px

What is the spacing at mobile?

Additional thoughts

Where does the 60px margin above the Heading 1 come from? I don't see it in the spacing for the individual Heading 1 component.
In looking at the website implementation the space above the Text introduction appears to be 45px
The Heading 1 component has a bottom margin of 15px that is being overruled by the 30px top margin from the Text introduction. Am I interpreting this correctly?

meissadia · 2023-10-12T23:05:57Z

@natalia-fitzgerald

Current spacing on desktop (with mobile spacing in perens)

<h1>
- top margin: 0px; -- YES (same on mobile)
- bottom-margin: 15px -- YES (same on mobile)
<div class="lead-paragraph">
- top-margin: 30px; -- YES (same on mobile)
- bottom-margin: 18.33px - Usually, unless the p.lead-paragraph is the last <p> in it's parent container (as we see in the Storybook example), in which case bottom-margin: 0;. The Lead Paragraph component is implemented as a <p>, not a <div> in the DS. Should it be a <div> instead? (15px on mobile)
<p>
- top margin: 0px; -- YES (same on mobile)
- bottom-margin: 15px -- Usually, unless it's the last <p> element in it's parent container (same on mobile)
Margin between H1 and Lead Paragraph: 30-px; -- YES (same on mobile)
Lead paragraph and Paragraph: 18.33px; -- YES (15px on mobile)
Paragraph and Link: 15px -- YES (same on mobile)

meissadia · 2023-10-12T23:18:52Z

@natalia-fitzgerald

Re: Additional thoughts

Where does the 60px margin above the Heading 1 come from? I don't see it in the spacing for the individual Heading 1 component.
- 60px top margin in the DSR's TextIntroduction component comes from the block class applied to the wrapper element that encases the TextIntroduction's elements (h1, p.lead-paragraph, p, .list-link). This mirrors the component composition seen on CF.gov
- That top margin of the block is removed on the CF.gov pages by adding the block_flush-top class (which can also be accomplished in DSR's TextIntroduction component by setting the isFlushTop property).
- The 45px top padding on CF.gov ex. Enforcement Actions comes from the page's grid layout component (.u-layout-grid_main) and does not come from the text introduction's elements.
In looking at the website implementation the space above the Text introduction appears to be 45px
- The 45px top padding on CF.gov ex. Enforcement Actions comes from the page's grid layout component (.u-layout-grid_main) and does not come from the text introduction's elements.
The Heading 1 component has a bottom margin of 15px that is being overruled by the 30px top margin from the Text introduction. Am I interpreting this correctly?
- Correct

meissadia · 2023-10-12T23:34:49Z

@natalia-fitzgerald

Re: Global Storybook style:

For paragraph text all Storybook pages (global) correct the line height. It is currently set to 24px. It should be 22px to match the paragraph styling.

This change would overwrite the styles for DS elements (ex: TextIntroduction's .lead-paragraph) which define their own line-height settings. Do we want to exclude such DS elements from this line height adjustment?

shindigira · 2023-10-13T20:06:37Z

Tested all functionality and tests. LGTM!

meissadia · 2023-10-13T23:16:51Z

Requesting re-review:

Implemented Natalia's feedback
Applied the the latest proposed TextIntroduction design

natalia-fitzgerald

@meissadia
Thanks for posting these changes.

The "Lead paragraph" within the "Paragraph" component page includes the top-margin of 30px. Is this because it's pulling in from the existing state in the CFPB DS? If so doesn't the DS also include a bottom-margin of 18.33px? Are we pulling in the DS lead paragraph or is our version modified?
The left navigation section is showing up with a light green background in the deploy preview. Can we change it back to white?
Will the 45px of space above the Text introduction component come from the parent page element?
Can we add the Text introduction component to the CFPB Design System and then connect it back to DSR? I posted an image with spacing specs here: Inconsistent margin in text introduction at desktop sizes design-system#1772 (comment)

natalia-fitzgerald · 2023-10-16T13:33:13Z

@meissadia
Would it be possible to show me the following in code (this will help with my review of the spacing depending on different combinations of the component parts)? Once the spacing is approved we can remove these extra combinations and just show the full component: - Heading 1 + Lead paragraph + Paragraph + Call-to-action link.

Can you show these combinations:

Heading 1 + Lead paragraph
Heading 1 + Lead paragraph + Paragraph
Heading 1 + Lead paragraph + Paragraph + Call-to-action link
Heading 1 + + Paragraph + Call-to-action link
Heading 1 + Lead paragraph + Call-to-action link

meissadia · 2023-10-24T22:52:03Z

Next up:

Remove CSS overrides for text-intro
Integrate updated DS styles
- Verify the .lead-paragraph top margin is removed
- Replace text-intro with the o-text-introduction class

natalia-fitzgerald · 2023-10-26T02:36:04Z

@meissadia
Can we make one small change to the Text introduction code sample (to match the CFPB Design System).

Here's the text I'd like to use:

Heading 1

Lead paragraph lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.

Descriptive paragraph lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore.

Call-to-action link

fix: Remove extraneous props for flushing tests: Update tests to use new test id

meissadia requested a review from natalia-fitzgerald October 5, 2023 17:25

meissadia requested a review from shindigira as a code owner October 5, 2023 17:25

meissadia requested a review from billhimmelsbach October 5, 2023 17:26

This was referenced Oct 5, 2023

[Component] Lead paragraph #157

Closed

[Component] Paragraph (Body) #162

Closed

natalia-fitzgerald requested changes Oct 10, 2023

View reviewed changes

natalia-fitzgerald changed the title ~~[Component] TextIntroduction~~ [Component] Text introduction Oct 10, 2023

shindigira approved these changes Oct 13, 2023

View reviewed changes

meissadia force-pushed the 158-text-introduction branch from 7e190ff to 4a94867 Compare October 13, 2023 22:51

meissadia requested review from shindigira and natalia-fitzgerald October 13, 2023 23:16

natalia-fitzgerald requested changes Oct 14, 2023

View reviewed changes

meissadia force-pushed the 158-text-introduction branch 5 times, most recently from 9986579 to ed7840b Compare October 16, 2023 19:55

meissadia added 6 commits October 24, 2023 16:28

feat: [Paragraph] Create component + tests + stories

79be7f5

feat: [TextIntroduction] Create component + tests + stories

98474e9

feat: [Storybook] Set default theme to "light" instead of "dark"

39311be

fix:[Paragraph] Remove duplicate intro paragraph

ed86d04

[Paragraph] Update placeholder text

0328bfa

fix: [TextIntroduction] Apply customized spacing to component elements

1af90a8

meissadia added 7 commits October 24, 2023 16:28

feat: [Storybook] Set line-height to 22px

734be8b

deps: Remove storybook-dark-mode

88e23e5

feat: [Storybook] Refactor and update UI theme

b5330f9

fix: [TextIntroduction] Apply latest component styles

e05db0c

docs: Note addtional places where CSS customization is implemented

7563dd2

fix: [Storybook] Update theme

d9552e8

WIP: Examples for spacing

2b21d79

meissadia force-pushed the 158-text-introduction branch from ed7840b to 2b21d79 Compare October 24, 2023 22:29

Merge branch 'main' into 158-text-introduction

ed160b5

meissadia added 4 commits October 26, 2023 12:35

Merge branch 'main' into 158-text-introduction

e31fbca

fix: Delete SpacingEvaluation stories

4876a33

fix: [TextIntroduction] Match DS language

de79742

fix: [TextIntroduction] Use DS styles

70f636a

fix: Remove extraneous props for flushing tests: Update tests to use new test id

meissadia force-pushed the 158-text-introduction branch from 03c8d9c to 70f636a Compare October 26, 2023 21:06

meissadia requested a review from natalia-fitzgerald October 26, 2023 21:14

natalia-fitzgerald approved these changes Oct 27, 2023

View reviewed changes

meissadia added 4 commits October 30, 2023 20:07

Merge branch 'main' into 158-text-introduction

80dffd5

Merge branch 'main' into 158-text-introduction

cd6a4a5

Merge branch 'main' into 158-text-introduction

39860f8

Move Paragraph and TextIntroduction to "Verified" status

da513ce

meissadia enabled auto-merge (squash) October 31, 2023 17:23

meissadia merged commit 116716a into main Oct 31, 2023
5 checks passed

meissadia deleted the 158-text-introduction branch October 31, 2023 17:26

meissadia mentioned this pull request Nov 9, 2023

Page-by-page: Shared Platform Components for CFPB Design System (in React) #224

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Component] Text introduction #193

[Component] Text introduction #193

meissadia commented Oct 5, 2023 •

edited

Loading

netlify bot commented Oct 5, 2023 •

edited

Loading

natalia-fitzgerald left a comment •

edited by meissadia

Loading

meissadia commented Oct 12, 2023

meissadia commented Oct 12, 2023

meissadia commented Oct 12, 2023

shindigira commented Oct 13, 2023

meissadia commented Oct 13, 2023

natalia-fitzgerald left a comment •

edited

Loading

natalia-fitzgerald commented Oct 16, 2023 •

edited

Loading

meissadia commented Oct 24, 2023 •

edited

Loading

natalia-fitzgerald commented Oct 26, 2023 •

edited

Loading

[Component] Text introduction #193

[Component] Text introduction #193

Conversation

meissadia commented Oct 5, 2023 • edited Loading

Changes

How to test this PR

Screenshots

netlify bot commented Oct 5, 2023 • edited Loading

✅ Deploy Preview for cfpb-design-system-react ready!

natalia-fitzgerald left a comment • edited by meissadia Loading

Choose a reason for hiding this comment

meissadia commented Oct 12, 2023

meissadia commented Oct 12, 2023

meissadia commented Oct 12, 2023

shindigira commented Oct 13, 2023

meissadia commented Oct 13, 2023

natalia-fitzgerald left a comment • edited Loading

Choose a reason for hiding this comment

natalia-fitzgerald commented Oct 16, 2023 • edited Loading

meissadia commented Oct 24, 2023 • edited Loading

natalia-fitzgerald commented Oct 26, 2023 • edited Loading

Heading 1

Lead paragraph lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.

meissadia commented Oct 5, 2023 •

edited

Loading

netlify bot commented Oct 5, 2023 •

edited

Loading

natalia-fitzgerald left a comment •

edited by meissadia

Loading

natalia-fitzgerald left a comment •

edited

Loading

natalia-fitzgerald commented Oct 16, 2023 •

edited

Loading

meissadia commented Oct 24, 2023 •

edited

Loading

natalia-fitzgerald commented Oct 26, 2023 •

edited

Loading