Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Adds new tokens to Tiles #134

Merged
merged 13 commits into from
Jun 29, 2022
Merged

Conversation

lucasfp13
Copy link
Contributor

@lucasfp13 lucasfp13 commented Jun 18, 2022

What's the purpose of this pull request?

This PR applies the new Theming structure to Tiles: new scoped tokens linked to global ones.

How does it work?

This PR uses local variables to parameterize the Tiles properties, then connects these scoped tokens to the global ones.

New global tokens:

None.

New local tokens for Tiles:

Default properties:

Local token Default value/Global token linked
--fs-tiles-gap-mobile var(--fs-grid-gap-2)
--fs-tiles-gap-tablet var(--fs-grid-gap-3)

Tile

Local token Default value/Global token linked
--fs-tiles-tile-min-width 9rem
--fs-tiles-tile-height 24rem
--fs-tiles-tile-height-desktop 28rem
--fs-tiles-tile-border-radius var(--fs-border-radius)

How to test it?

The component should look just as it was before these changes.

Checklist

Changelog

  • 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 (#4)

PR Description

  • 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.

Documentation

  • PR description
  • Added to/Updated the Storybook - if applicable.

@lucasfp13 lucasfp13 added the Chore General tasks. label Jun 18, 2022
@lucasfp13 lucasfp13 self-assigned this Jun 18, 2022
@vercel
Copy link

vercel bot commented Jun 18, 2022

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

Name Status Preview Updated
nextjs-store-storybook ✅ Ready (Inspect) Visit Preview Jun 28, 2022 at 10:29PM (UTC)

@vercel vercel bot temporarily deployed to preview June 18, 2022 17:16 Inactive
@vtex-sites
Copy link

vtex-sites bot commented Jun 18, 2022

Preview is ready

This pull request generated a Preview

👀   Preview: https://sfj-b9ab9da--nextjs.preview.vtex.app
🔬   Go deeper by inspecting the Build Logs
📝   based on commit b9ab9da

@lucasfp13 lucasfp13 marked this pull request as ready for review June 21, 2022 16:31
@vercel vercel bot temporarily deployed to preview June 21, 2022 16:32 Inactive
@vercel vercel bot temporarily deployed to preview June 22, 2022 18:46 Inactive
@vercel vercel bot temporarily deployed to preview June 22, 2022 19:00 Inactive
@lucasfp13 lucasfp13 force-pushed the feat/FSSS-266-adds-new-tokens-to-tiles branch from de13213 to ceff644 Compare June 22, 2022 19:21
@vercel vercel bot temporarily deployed to preview June 22, 2022 19:23 Inactive
@vtex-sites
Copy link

vtex-sites bot commented Jun 23, 2022

Lighthouse Reports

Here are the Lighthouse reports of this Pull Request

📝 Based on commit b9ab9da

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

Copy link
Contributor

@filipewl filipewl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me overall!
👏

Left some minor comments and suggestions for us to discuss further.

src/components/ui/Tiles/Tiles.stories.mdx Outdated Show resolved Hide resolved
src/components/ui/Tiles/Tiles.stories.mdx Outdated Show resolved Hide resolved
src/components/ui/Tiles/Tiles.stories.mdx Outdated Show resolved Hide resolved
src/components/ui/Tiles/tiles.module.scss Outdated Show resolved Hide resolved
src/components/ui/Tiles/tiles.module.scss Outdated Show resolved Hide resolved
src/components/ui/Tiles/Tiles.stories.mdx Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to preview June 23, 2022 18:38 Inactive
@lucasfp13 lucasfp13 force-pushed the feat/FSSS-266-adds-new-tokens-to-tiles branch from e86de80 to 1e30433 Compare June 23, 2022 19:35
@vercel vercel bot temporarily deployed to preview June 23, 2022 19:37 Inactive
Copy link
Contributor

@hellofanny hellofanny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!! 🚀

I've left small suggestions~

@vercel vercel bot temporarily deployed to preview June 24, 2022 01:10 Inactive
@lucasfp13 lucasfp13 force-pushed the feat/FSSS-266-adds-new-tokens-to-tiles branch from 20ec53c to 09cdda9 Compare June 24, 2022 01:35
@vercel vercel bot temporarily deployed to preview June 24, 2022 01:36 Inactive
@vercel vercel bot temporarily deployed to preview June 27, 2022 13:40 Inactive
@lucasfp13 lucasfp13 force-pushed the feat/FSSS-266-adds-new-tokens-to-tiles branch from 4d14cb7 to 955e4cf Compare June 28, 2022 13:48
@vercel vercel bot temporarily deployed to preview June 28, 2022 13:50 Inactive
Copy link
Collaborator

@renatamottam renatamottam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

src/components/ui/Tiles/Tiles.stories.mdx Outdated Show resolved Hide resolved
src/components/ui/Tiles/Tiles.stories.mdx Show resolved Hide resolved
@lucasfp13 lucasfp13 force-pushed the feat/FSSS-266-adds-new-tokens-to-tiles branch from bab163c to 4a53b5b Compare June 28, 2022 22:27
@vercel vercel bot temporarily deployed to preview June 28, 2022 22:29 Inactive
@lucasfp13 lucasfp13 merged commit b9ab9da into main Jun 29, 2022
@lucasfp13 lucasfp13 deleted the feat/FSSS-266-adds-new-tokens-to-tiles branch June 29, 2022 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chore General tasks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants