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: [Refinements] Creates ShippingSimulation #183

Merged
merged 27 commits into from
Aug 16, 2022

Conversation

eduardoformiga
Copy link
Member

@eduardoformiga eduardoformiga commented Jul 27, 2022

What's the purpose of this pull request?

This PR aims to create and refine the ShippingSimulation that is found in the PDP.
This task didn't include API integration. Thus, we are using mocked data just to see the component working as expected.


Tasks:

  • Creates Shipping Simulation.
  • CSS Modules.
  • Themification.
  • Storybook.

How does it work?

Desktop Mobile
06VzGVSK2e mobile-shipping-simulation

👀 One behavior to pay attention to:

  • if the user had a PostalCode included in its session, the page should load this PostalCode in the ShippingSimulation as the default.
  • if the user sets the PostalCode in its session while in the PDP using the Regionalization feature, the page should load this PostalCode only if there is no PostalCode included in the ShippingSimulation. But as we don't save the PostalCode from the ShippingSimulation, if the user refreshes the page, the one from the session must override the other from the ShippingSimulation.

7rxQ0BdqYo

How to test it?

So far, we are using mocked data, but you can play around inside the PDP. If something didn't work properly (because we are using a user session), try to clean the IndexDB data.

You can also see the storybook.

References

https://www.figma.com/file/EYJqnbWByrSX9EMKvBzBLm/22%2F06%2F14-Handoff-for-PDP-Components?node-id=1001%3A131786
https://www.figma.com/file/EYJqnbWByrSX9EMKvBzBLm/22%2F06%2F14-Handoff-for-PDP-Components?node-id=1001%3A152727
https://vtex-dev.atlassian.net/jira/software/projects/FS/boards/632?selectedIssue=FS-475

Checklist

You may erase this after checking them all ;)

Changelog

  • Added an entry in the CHANGELOG.md at the beginning of its due section. The latest version should come 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.
  • For documentation changes, ping @ carolinamenezes or @ PedroAntunesCosta to review and update.

@vercel
Copy link

vercel bot commented Jul 27, 2022

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

Name Status Preview Updated
nextjs-store-storybook ✅ Ready (Inspect) Visit Preview Aug 16, 2022 at 3:18PM (UTC)

@vercel vercel bot temporarily deployed to Preview July 27, 2022 00:43 Inactive
@vtex-sites
Copy link

vtex-sites bot commented Jul 27, 2022

Preview is ready

This pull request generated a Preview

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

@vercel vercel bot temporarily deployed to Preview July 28, 2022 01:40 Inactive
@vercel vercel bot temporarily deployed to Preview July 28, 2022 18:16 Inactive
@vercel vercel bot temporarily deployed to Preview July 28, 2022 20:00 Inactive
@eduardoformiga eduardoformiga changed the title Feat: Refinement/Creates Shipping Simulation Feat: Refinement/Creates ShippingSimulation Jul 28, 2022
@eduardoformiga eduardoformiga added Documentation Improvements or additions to documentation Features New feature or request labels Jul 28, 2022
@vercel vercel bot temporarily deployed to Preview July 28, 2022 21:32 Inactive
@vtex-sites
Copy link

vtex-sites bot commented Jul 28, 2022

Lighthouse Reports

Here are the Lighthouse reports of this Pull Request

📝 Based on commit 64c74f6

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

@vercel vercel bot temporarily deployed to Preview July 28, 2022 22:27 Inactive
@eduardoformiga eduardoformiga changed the title Feat: Refinement/Creates ShippingSimulation Feat: [Component Refinements] Creates ShippingSimulation Jul 28, 2022
@vercel vercel bot temporarily deployed to Preview July 28, 2022 23:22 Inactive
@eduardoformiga eduardoformiga changed the title Feat: [Component Refinements] Creates ShippingSimulation Feat: [Refinements] Creates ShippingSimulation Jul 28, 2022
@eduardoformiga eduardoformiga marked this pull request as ready for review July 28, 2022 23:29
@vercel vercel bot temporarily deployed to Preview August 15, 2022 21:19 Inactive
@eduardoformiga
Copy link
Member Author

eduardoformiga commented Aug 15, 2022

should this blank space exist on the desktop? shouldn't the height of the space be equal to the height of the input + padding?

Screen Shot 2022-08-02 at 10 41 52

You are right! I have tried different approaches to solve this.

It's a CSS grid problem that involves the ProductGallery as well, and if we change the shipping simulation row it will affect the Product Image size. Since we are going to have other components like SkuSelector I think it's better to adjust the whole grid after finishing all components and using another task for it. So far, these sizes look acceptable without having to spend more time on something that will change. What do you think?

Copy link
Contributor

@tlgimenes tlgimenes left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -36,6 +36,7 @@ type ActionableInputText =
onSubmit?: never
onClear?: never
buttonActionText?: string
displayClearButton?: never
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL never

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.

I've tested it on the PDP page, and looks great! 🚀
I've left just a few things to discuss :)
I was not able to see it in the storybook. Something seems to be broken there. Can you check it, pls?

@vercel vercel bot temporarily deployed to Preview August 16, 2022 14:59 Inactive
@vercel vercel bot temporarily deployed to Preview August 16, 2022 15:06 Inactive
@vercel vercel bot temporarily deployed to Preview August 16, 2022 15:10 Inactive
@vercel vercel bot temporarily deployed to Preview August 16, 2022 15:18 Inactive
@eduardoformiga
Copy link
Member Author

eduardoformiga commented Aug 16, 2022

I was not able to see it in the storybook. Something seems to be broken there. Can you check it, pls?

Well observed! That's because after rebase with main and getting #157 updates, we don't have the SessionProvider anymore. So I just remove it and it works again 😉

@eduardoformiga eduardoformiga merged commit 35aca51 into main Aug 16, 2022
@eduardoformiga eduardoformiga deleted the feat/fs-475-shipping-simulation branch August 16, 2022 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements or additions to documentation Features New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants