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.

Sign up for GitHub

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

Jump to bottom

Feat: Theming Adjustments #175

Merged
merged 21 commits into from
Jul 22, 2022
Merged

Feat: Theming Adjustments #175

merged 21 commits into from
Jul 22, 2022

Conversation

renatamottam
Copy link
Collaborator

@renatamottam renatamottam commented Jul 18, 2022 •
edited
Loading

What's the purpose of this pull request?

This PR addresses some adjustments proposed by @eduardoformiga on #119

  • Fixes SectionListProps
  • Fixes Tiles border-radius
  • Adds missing TokenColor on Interactive Controls page
  • Removes commented tokens from custom-theme
  • Fixes theme class on body
  • Fixes CartToggle background color
  • Removes Themes dependencies
  • Creates Themes pages on Storybook

How to test it?

Use the links for Storybook and Live Store.
Most important pages to review:

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.
  • Identified the function or parameter in the PR - If applicable. E.g., useWindowDimensions hook.

Documentation

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

@vercel
Copy link

vercel bot commented Jul 18, 2022 •
edited
Loading

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

Name Status Preview Updated
nextjs-store-storybook ✅ Ready (Inspect) Visit Preview Jul 22, 2022 at 0:41AM (UTC)

@vtex-sites
Copy link

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

Preview is ready

This pull request generated a Preview

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

@vercel vercel bot temporarily deployed to Preview July 18, 2022 22:22 Inactive
@renatamottam renatamottam self-assigned this Jul 18, 2022
@renatamottam renatamottam added the Features New feature or request label Jul 18, 2022
@vtex-sites
Copy link

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

Lighthouse Reports

Here are the Lighthouse reports of this Pull Request

📝 Based on commit e9277ba

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

@hellofanny
Copy link
Contributor

@renatamottam I have 2 suggestions for the tokens, maybe it could be addressed here:

  1. These 3 tokens (--fs-text-size-base, --fs-text-scale-desk, --fs-text-scale-mobile) could be listed in another table
    Screenshot (3)

  2. Maybe we could use the following pattern when naming tokens for breakpoints:

breakpoint shortcut
mobile -mob
tablet -tab or -tablet
desktop -desk
notebook -note

What do you think?

@vercel vercel bot temporarily deployed to Preview July 20, 2022 14:14 Inactive
@renatamottam renatamottam marked this pull request as ready for review July 20, 2022 14:42
@vercel vercel bot temporarily deployed to Preview July 20, 2022 14:43 Inactive
@renatamottam renatamottam added the Documentation Improvements or additions to documentation label Jul 20, 2022
@renatamottam renatamottam requested review from eduardoformiga and lucasfp13 and removed request for eduardoformiga July 20, 2022 14:59
@vercel vercel bot temporarily deployed to Preview July 20, 2022 15:04 Inactive
eduardoformiga
eduardoformiga approved these changes Jul 20, 2022
View reviewed changes
Copy link
Member

@eduardoformiga eduardoformiga left a comment

Choose a reason for hiding this comment

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

Awesome! 🎉

@renatamottam renatamottam requested a review from hellofanny July 21, 2022 12:34
@vercel vercel bot temporarily deployed to Preview July 21, 2022 12:36 Inactive
@vercel vercel bot temporarily deployed to Preview July 21, 2022 20:08 Inactive
hellofanny
hellofanny reviewed Jul 22, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
hellofanny
hellofanny approved these changes Jul 22, 2022
View reviewed changes
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.

Awesomee job 🌟🌟🌟

Renata Motta and others added 2 commits July 22, 2022 09:37
@hellofanny
Updates Customizing copy
8aed5df 
Co-authored-by: Fanny Chien <[email protected]>
@hellofanny
Updates Changelog
e9277ba 
Co-authored-by: Fanny Chien <[email protected]>
@vercel vercel bot temporarily deployed to Preview July 22, 2022 12:41 Inactive
@renatamottam renatamottam merged commit d3fe885 into main Jul 22, 2022
@renatamottam renatamottam deleted the feat/theming-adjustments branch July 22, 2022 12:47
@renatamottam renatamottam mentioned this pull request Jul 22, 2022
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eduardoformiga eduardoformiga eduardoformiga approved these changes

@hellofanny hellofanny hellofanny approved these changes

@lucasfp13 lucasfp13 Awaiting requested review from lucasfp13

@ArthurTriis1 ArthurTriis1 Awaiting requested review from ArthurTriis1

Assignees

@renatamottam renatamottam

Labels
Documentation Improvements or additions to documentation Features New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@renatamottam @hellofanny @eduardoformiga