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

Product Form Abstraction #1027

Merged
merged 13 commits into from Jul 6, 2023
Merged

Product Form Abstraction #1027

merged 13 commits into from Jul 6, 2023

Conversation

blittle
Copy link
Contributor

@blittle blittle commented Jun 22, 2023

WHY are these changes introduced?

Implements: https://github.com/Shopify/hydrogen-internal/issues/23

See https://gist.github.com/blittle/d9205d4ac72528005dc6f3104c328ecd for a public explanation.

WHAT is this pull request doing?

HOW to test your changes?

Post-merge steps

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes
  • I've added or updated the documentation

Docs WIP: https://shopify-dev.shopify-dev-4mgc.bret-little.us.spin.dev/docs/api/hydrogen/2023-04/unstable/components/variantselector

@benjaminsehl
Copy link
Member

@@ -220,10 +211,103 @@ export function ProductForm() {
return (
<div className="grid gap-10">
<div className="grid gap-4">
<ProductOptions
<VariantSelector
options={product.options}
Copy link
Member

Choose a reason for hiding this comment

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

Do we need options and variants separated out? Shouldn't we just have product={product} and we just derive options and variants from that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like having them separate, so that they can come from separate requests. Only options is required. The variants could be deferred.

@benjaminsehl
Copy link
Member

@github-actions

This comment has been minimized.

@blittle blittle changed the title WIP Product Form Abstraction Product Form Abstraction Jun 26, 2023
@github-actions github-actions bot had a problem deploying to preview July 5, 2023 22:41 Failure
@blittle blittle merged commit 1befd36 into 2023-04 Jul 6, 2023
10 checks passed
@blittle blittle deleted the bl-product-form branch July 6, 2023 15:33
juanpprieto pushed a commit that referenced this pull request Jul 10, 2023
FrcPpe pushed a commit to FrcPpe/hydrogen that referenced this pull request Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants