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: Add new tokens to SearchInput [FS-496] #189

Merged
merged 6 commits into from
Aug 8, 2022
Merged

feat: Add new tokens to SearchInput [FS-496] #189

merged 6 commits into from
Aug 8, 2022

Conversation

lucasfp13
Copy link
Contributor

@lucasfp13 lucasfp13 commented Aug 2, 2022 •
edited
Loading

What's the purpose of this pull request?

This PR intends to apply new tokens to SearchInput based on the new Theming structure: new scoped tokens linked to global ones.

How does it work?

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

How to test it?

These changes should not affect the SearchInput behavior. It can be tested through Storybook, inside the Search feature page.

Checklist

PR Title and Commit Messages

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.

@lucasfp13 lucasfp13 added the Features New feature or request label Aug 2, 2022
@lucasfp13 lucasfp13 self-assigned this Aug 2, 2022
@vercel
Copy link

vercel bot commented Aug 2, 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 Aug 5, 2022 at 1:04PM (UTC)

@vercel vercel bot temporarily deployed to Preview August 2, 2022 12:22 Inactive
@vtex-sites
Copy link

vtex-sites bot commented Aug 2, 2022 •
edited
Loading

Preview is ready

This pull request generated a Preview

👀   Preview: https://preview-189--nextjs.preview.vtex.app
🔬   Go deeper by inspecting the Build Logs
📝   based on commit 3e9bbe4
⚠️   User login might not work on this preview.

@vtex-sites
Copy link

vtex-sites bot commented Aug 2, 2022 •
edited
Loading

Lighthouse Reports

Here are the Lighthouse reports of this Pull Request

📝 Based on commit 3e9bbe4

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

@lucasfp13 lucasfp13 force-pushed the feat/FS-496-add-new-tokens-to-search-input branch from 284e7ff to 3071b60 Compare August 2, 2022 12:33
@lucasfp13 lucasfp13 marked this pull request as ready for review August 2, 2022 12:33
@vercel vercel bot temporarily deployed to Preview August 2, 2022 12:36 Inactive
filipewl
filipewl approved these changes Aug 2, 2022
View reviewed changes
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.

Awesome! It's still looking the same.

@lucasfp13 lucasfp13 changed the title feat: Add new tokens to SearchInput feat: Add new tokens to SearchInput [FS-496] Aug 3, 2022
@lucasfp13 lucasfp13 force-pushed the feat/FS-496-add-new-tokens-to-search-input branch from 3071b60 to 96ed414 Compare August 3, 2022 19:48
@vercel vercel bot temporarily deployed to Preview August 3, 2022 19:52 Inactive
@vercel vercel bot temporarily deployed to Preview August 3, 2022 20:15 Inactive
hellofanny
hellofanny requested changes Aug 4, 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.

Well done!! It's working fine 🚀

I've just noticed an issue with this transition token name. I think I've suggested changes in all of the occurrences. After fixing it, I guess LGTM, ok?

@lucasfp13 lucasfp13 force-pushed the feat/FS-496-add-new-tokens-to-search-input branch from 79413ab to 22a3613 Compare August 4, 2022 17:21
@vercel vercel bot temporarily deployed to Preview August 4, 2022 17:25 Inactive
@lucasfp13 lucasfp13 force-pushed the feat/FS-496-add-new-tokens-to-search-input branch from 22a3613 to 348cdf2 Compare August 4, 2022 18:24
@lucasfp13 lucasfp13 requested a review from hellofanny August 4, 2022 18:25
@vercel vercel bot temporarily deployed to Preview August 4, 2022 18:31 Inactive
hellofanny
hellofanny approved these changes Aug 5, 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.

🌟

@vercel vercel bot temporarily deployed to Preview August 5, 2022 13:04 Inactive
@lucasfp13 lucasfp13 merged commit f682090 into main Aug 8, 2022
@lucasfp13 lucasfp13 deleted the feat/FS-496-add-new-tokens-to-search-input branch August 8, 2022 17:28
lucasfp13 added a commit to vtex-sites/gatsby.store that referenced this pull request Aug 12, 2022
@lucasfp13
chore: apply patch from vtex-sites/nextjs.store#189
fbe372f
@lucasfp13 lucasfp13 mentioned this pull request Aug 12, 2022
Merged
7 tasks
lucasfp13 added a commit to vtex-sites/gatsby.store that referenced this pull request Aug 12, 2022
@lucasfp13
chore: apply patch from vtex-sites/nextjs.store#189
7ada5da
lucasfp13 added a commit to vtex-sites/gatsby.store that referenced this pull request Aug 12, 2022
@lucasfp13
chore: apply patch from vtex-sites/nextjs.store#189
cf54702
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@renatamottam renatamottam renatamottam left review comments

@hellofanny hellofanny hellofanny approved these changes

@filipewl filipewl filipewl approved these changes

Assignees

@lucasfp13 lucasfp13

Labels
Features New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lucasfp13 @filipewl @hellofanny @renatamottam