Feat: Adds new tokens to Incentives #49

saranicoly · 2022-05-18T12:41:21Z

What's the purpose of this pull request?

This PR intends to add new tokens to the Incentives component using the Theming Structure.
Corresponding PR in Nextjs store: vtex-sites/nextjs.store#56

How does it work?

This PR uses local variables to parameterize the Incentives properties, then connects these scoped tokens to the global ones.
It also adds a new variant to Incentives, which can be called like that:
< Incentives incentives={IncentivesMock} variant="vertical" />

New global tokens:

None.

New local tokens for `Incentives`:

Default properties:

Local token	Default value/Global token linked
`--fs-incentives-bkg-color`	`var(--fs-color-primary-bkg-light)`
`--fs-incentives-gap`	`var(--fs-spacing-4)`
`--fs-incentives-padding-top`	`var(--fs-incentives-gap)`
`--fs-incentives-padding-bottom`	`var(--fs-incentives-gap)`
`--fs-incentives-border-color`	`var(--fs-border-color-light)`
`--fs-incentives-border-width`	`var(--fs-border-width)`

Nested Elements:

Title

Local token	Default value/Global token linked
`--fs-incentives-title-size`	`var(--fs-text-size-1)`
`--fs-incentives-title-weight`	`var(--fs-text-weight-bold)`
`--fs-incentives-title-line-height`	`1.42`
`--fs-incentives-title-color`	`var(--fs-color-neutral-7)`

Description

Local token	Default value/Global token linked
`--fs-incentives-description-size`	`var(--fs-incentives-title-size)`
`--fs-incentives-description-line-height`	`1.14`
`--fs-incentives-description-color`	`var(--fs-incentives-title-color)`

Icon

Local token	Default value/Global token linked
`--fs-incentives-icon-color`	`var(--fs-incentives-title-color)`

How to test it?

The Incentives component should look just as it was before these changes. You may see it either on Gatsby store and on the Storybook.

Checklist

You may erase this after checking them all ;)

Added an entry in the CHANGELOG.md at the beginning of its due section. The latest version should comes first.
Added the PR number with the PR link at the entry in the CHANGELOG.md. E.g., New items in the pull_request_template.md (#12)
PR description
Updated the Storybook - if applicable.
Added a label according to the PR goal - Breaking change, Enhancement, Bug or Chore.
Added the component, hook, or pathname in-between backticks (``) - If applicable. E.g., ComponentName component.
Identified the function or parameter in the PR - If applicable. E.g., useWindowDimensions hook.
For documentation changes, ping @carolinamenezes or @Mariana-Caetano to review and update the changes.

vercel · 2022-05-18T12:41:24Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
gatsby-store-storybook	✅ Ready (Inspect)	Visit Preview	May 23, 2022 at 10:22PM (UTC)

vtex-sites · 2022-05-18T12:45:10Z

Preview is ready

This pull request generated a Preview

👀 Preview: https://preview-49--gatsby.preview.vtex.app
🔬 Go deeper by inspecting the Build Logs
📝 based on commit 68266c0

vtex-sites · 2022-05-18T12:50:13Z

Lighthouse Reports

Here are the Lighthouse reports of this Pull Request

📝 Based on commit 68266c0

Lighthouse Report by page
📎 /
📎 /apple-magic-mouse-99988212/p
📎 /office

…s/gatsby.store into feat/theming-incentive

Try to improve the page LCP by boosting the priority for the main image. By setting `fetchpriority` to `high` we can hint the browser to give a higher priority to our LCP images. Use `psi-test` to compare before and after. Page result - https://sfj-7de89f9--nextjs.preview.vtex.app/ | Metric | Mean | Standard deviation | Confidence Interval (95%) | |--------|--------|--------|--------| | Largest Contentful Paint (LCP) | 2221.49 | 152192.99 | [-64480.03, 68923.01] | | Performance score | 0.839 | 0.000979 | [0.838571, 0.839429] | Page result - https://sfj-f5d2958--nextjs.preview.vtex.app/ | Metric | Mean | Standard deviation | Confidence Interval (95%) | |--------|--------|--------|--------| | Largest Contentful Paint (LCP) | 1475.76 | 42521.48 | [-17160.10, 20111.61] | | Performance score | 0.879 | 0.000619 | [0.878729, 0.879271] | Page result - https://sfj-7de89f9--nextjs.preview.vtex.app/office | Metric | Mean | Standard deviation | Confidence Interval (95%) | |--------|--------|--------|--------| | Largest Contentful Paint (LCP) | 1681.58 | 120980.33 | [-51340.39, 54703.55] | | Performance score | 0.899 | 0.000625 | [0.899226, 0.899774] | Page result - https://sfj-f5d2958--nextjs.preview.vtex.app/office | Metric | Mean | Standard deviation | Confidence Interval (95%) | |--------|--------|--------|--------| | Largest Contentful Paint (LCP) | 1422.32 | 55090.06 | [-22721.96, 25566.61] | | Performance score | 0.903 | 0.000169 | [0.902426, 0.902574] | Page result - https://sfj-7de89f9--nextjs.preview.vtex.app/apple-magic-mouse-99988212/p | Metric | Mean | Standard deviation | Confidence Interval (95%) | |--------|--------|--------|--------| | Largest Contentful Paint (LCP) | 2855.38 | 65371.98 | [-25795.15, 31505.92] | | Performance score | 0.736 | 0.005824 | [0.733448, 0.738552] | Page result - https://sfj-f5d2958--nextjs.preview.vtex.app/apple-magic-mouse-99988212/p | Metric | Mean | Standard deviation | Confidence Interval (95%) | |--------|--------|--------|--------| | Largest Contentful Paint (LCP) | 2294.23 | 84993.06 | [-34955.62, 39544.08] | | Performance score | 0.842 | 0.003156 | [0.840617, 0.843383] | - [[web.dev] Optimizing resource loading with Priority Hints](https://web.dev/priority-hints/) - [[web.dev] Optimize Largest Contentful Paint](https://web.dev/optimize-lcp/)

…s/gatsby.store into feat/theming-incentive

filipewl

Nice job! 👏

filipewl · 2022-05-23T17:35:00Z

src/components/sections/Incentives/incentives.scss

+  --fs-incentives-title-line-height    : 1.42;
+
+  // Description
+  --fs-incentives-description-size          : var(--fs-incentives-title-size);
+  --fs-incentives-description-line-height   : 1.14;


@renatamottam and @hellofanny, are you planning to have tokens for the line-height? It would be nice to standardize some values in tokens so they don't feel arbitrary.

I don't think so. We have associate line-height for titles on Typography, not sure if this works for local values. But might be a good idea.

…s/gatsby.store into feat/theming-incentive

src/components/sections/Incentives/Incentives.stories.mdx

hellofanny

Looks great! 🤩 🤩 🤩 I noticed that Props section title was missing on the doc, so I've left it as a suggestion.

Co-authored-by: Fanny Chien <fannychienn@gmail.com>

add scope to storybook

3bc82fe

vercel bot deployed to Preview May 18, 2022 12:41 View deployment

renatamottam assigned saranicoly May 19, 2022

renatamottam changed the title ~~feat: Adds new tokens to Link~~ Feat: Adds new tokens to Incentives May 19, 2022

saranicoly added 2 commits May 19, 2022 09:55

add scope to storybook

51f38d6

Merge branch 'feat/theming-incentive' of https://github.com/vtex-site…

2323cc5

…s/gatsby.store into feat/theming-incentive

vercel bot deployed to Preview May 19, 2022 13:00 View deployment

Add tokens [WIP]

a4f3eb9

vercel bot deployed to Preview May 19, 2022 13:57 View deployment

add tokens [WIP]

7660af6

add vertical variant

725277f

vercel bot deployed to Preview May 19, 2022 18:03 View deployment

saranicoly changed the base branch from main to chore/fsss-306-add-optional-dependencies May 19, 2022 18:06

saranicoly changed the base branch from chore/fsss-306-add-optional-dependencies to main May 19, 2022 18:06

saranicoly added 2 commits May 19, 2022 15:08

remove initial changes from storybook

5800ba4

add storybook

2a98bb2

vercel bot had a problem deploying to Preview May 19, 2022 21:26 Failure

add basic storybook

806dd1a

vercel bot deployed to Preview May 20, 2022 17:29 View deployment

finish storybook

e8bcbff

vercel bot deployed to Preview May 20, 2022 18:08 View deployment

saranicoly added the Chore General tasks. label May 20, 2022

saranicoly added 4 commits May 20, 2022 15:24

add scope to storybook

7b30494

Add tokens [WIP]

68220f9

add tokens [WIP]

73a095a

add vertical variant

7be4195

saranicoly added 5 commits May 20, 2022 15:24

remove initial changes from storybook

3ec8bc0

add storybook

37565ce

add basic storybook

f654f5b

finish storybook

de97246

Merge branch 'feat/theming-incentive' of https://github.com/vtex-site…

284f7ba

…s/gatsby.store into feat/theming-incentive

vercel bot deployed to Preview May 20, 2022 18:25 View deployment

adds changelog

41824c6

vercel bot deployed to Preview May 20, 2022 18:27 View deployment

saranicoly marked this pull request as ready for review May 20, 2022 18:30

Merge branch 'main' into feat/theming-incentive

f2f3771

vercel bot deployed to Preview May 23, 2022 17:03 View deployment

filipewl approved these changes May 23, 2022

View reviewed changes

saranicoly added 2 commits May 23, 2022 16:47

add tokens

f7a9631

Merge branch 'feat/theming-incentive' of https://github.com/vtex-site…

a39bd5b

…s/gatsby.store into feat/theming-incentive

vercel bot deployed to Preview May 23, 2022 19:49 View deployment

add new tokens to storybook

b20e975

vercel bot deployed to Preview May 23, 2022 20:26 View deployment

fix alignment

767abae

vercel bot deployed to Preview May 23, 2022 20:28 View deployment

saranicoly requested review from hellofanny and renatamottam May 23, 2022 20:32

hellofanny reviewed May 23, 2022

View reviewed changes

src/components/sections/Incentives/Incentives.stories.mdx Show resolved Hide resolved

hellofanny approved these changes May 23, 2022

View reviewed changes

add props title to storybook

324598d

Co-authored-by: Fanny Chien <fannychienn@gmail.com>

vercel bot deployed to Preview May 23, 2022 22:22 View deployment

saranicoly mentioned this pull request May 23, 2022

Feat: Adds new tokens to Incentives vtex-sites/nextjs.store#56

Merged

7 tasks

saranicoly merged commit 68266c0 into main May 24, 2022

saranicoly deleted the feat/theming-incentive branch May 24, 2022 13:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Adds new tokens to Incentives #49

Feat: Adds new tokens to Incentives #49

saranicoly commented May 18, 2022 •

edited

Loading

vercel bot commented May 18, 2022 •

edited

Loading

vtex-sites bot commented May 18, 2022 •

edited

Loading

vtex-sites bot commented May 18, 2022 •

edited

Loading

filipewl left a comment

filipewl May 23, 2022

hellofanny May 23, 2022

hellofanny left a comment

Feat: Adds new tokens to Incentives #49

Feat: Adds new tokens to Incentives #49

Conversation

saranicoly commented May 18, 2022 • edited Loading

What's the purpose of this pull request?

How does it work?

New global tokens:

New local tokens for Incentives:

Default properties:

Nested Elements:

Title

Description

Icon

How to test it?

Checklist

vercel bot commented May 18, 2022 • edited Loading

vtex-sites bot commented May 18, 2022 • edited Loading

Preview is ready

This pull request generated a Preview

vtex-sites bot commented May 18, 2022 • edited Loading

Lighthouse Reports

Here are the Lighthouse reports of this Pull Request

filipewl left a comment

Choose a reason for hiding this comment

filipewl May 23, 2022

Choose a reason for hiding this comment

hellofanny May 23, 2022

Choose a reason for hiding this comment

hellofanny left a comment

Choose a reason for hiding this comment

saranicoly commented May 18, 2022 •

edited

Loading

New local tokens for `Incentives`:

vercel bot commented May 18, 2022 •

edited

Loading

vtex-sites bot commented May 18, 2022 •

edited

Loading

vtex-sites bot commented May 18, 2022 •

edited

Loading