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(dashboard): restructure create product flow #7374

Merged
merged 38 commits into from
May 28, 2024

Conversation

fPolic
Copy link
Contributor

@fPolic fPolic commented May 20, 2024

WHAT

  • restructure the product create flow

Other

  • fix pricing formating on create/edit
  • make edit pricing form work properly

NOTE

  • inventory kit tab is incomplete ATM until support is added to the workflow

Copy link

vercel bot commented May 20, 2024

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

Name Status Preview Comments Updated (UTC)
medusa-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 28, 2024 9:08am
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
api-reference ⬜️ Ignored (Inspect) May 28, 2024 9:08am
docs-ui ⬜️ Ignored (Inspect) Visit Preview May 28, 2024 9:08am
medusa-docs ⬜️ Ignored (Inspect) Visit Preview May 28, 2024 9:08am

Copy link

changeset-bot bot commented May 20, 2024

⚠️ No Changeset found

Latest commit: 06e89d1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@olivermrbl
Copy link
Contributor

Very nice!

Minor thing: We should default to having all variants checked in the first step of the flow

@olivermrbl
Copy link
Contributor

Also, another thing: When selecting "No variants", we should create a default variant under the hood. The details should just say "Default variant" etc.

I should have mentioned this earlier, my bad.

@fPolic
Copy link
Contributor Author

fPolic commented May 21, 2024

Minor thing: We should default to having all variants checked in the first step of the flow

@olivermrbl should this be for the first option only or? What if the user creates an option, selects only some variants for creation and then creates a new option, which variants are checked then?

@olivermrbl
Copy link
Contributor

olivermrbl commented May 21, 2024

If we can have a control flow like so, that would be great:

Scenario 1:

  • Create option
  • *All variants are selected

Scenario 2:

  • Create option
  • *All variants are selected
  • Select only two variants
  • Create new option
  • The same two remain selected

Scenario 3:

  • Create option
  • *All variants are selected
  • Create new option
  • *All variants (old + new) are selected

@fPolic
Copy link
Contributor Author

fPolic commented May 21, 2024

Also, another thing: When selecting "No variants", we should create a default variant under the hood. The details should just say "Default variant" etc.

@olivermrbl we create that on the admin, right? Do we create the default variant right after the first step is complete or when we send the payload? Because if we create a default variant for the user should we also create default pricing and inventory stuff for that variant?

@olivermrbl
Copy link
Contributor

we create that on the admin, right? Do we create the default variant right after the first step is complete or when we send the payload? Because if we create a default variant for the user should we also create default pricing and inventory stuff for that variant?

Yeah, we should still allow merchants to specify pricing and inventory for the default variant. Something like this. Lemme just validate the design with Ludvig

@fPolic fPolic changed the title feat(dashboard): restructure create product details flow feat(dashboard): restructure create product flow May 22, 2024
@fPolic
Copy link
Contributor Author

fPolic commented May 27, 2024

@olivermrbl @kasperkristensen would love to hear your thoughts on this one

@olivermrbl
Copy link
Contributor

Will give this a spin today!

Are there any larger todos remaining?

@fPolic
Copy link
Contributor Author

fPolic commented May 27, 2024

No, should be ready for review

Copy link
Contributor

@kasperkristensen kasperkristensen left a comment

Choose a reason for hiding this comment

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

Couple of things:

We need to initialize the values of each field when we create a variant in-memory. If not we get two issues: The input changes from uncontrolled to controlled, and if you add a value to a field that isn't inited, and then hit cmd + z it will insert the value "undefined" into the field.

The grid gets laggy when you have many variants, and you are horizontally scrolled so all of the boolean cells are rendered. I saw a good amount of performance increase when I changed the country select cell to render a placeholder when its not the anchor cell. Think we should do this for all "complex" cells, but fine to leave for now, as the grid is still WIP (need to address the things I've outlined in the comment above the component).

@fPolic
Copy link
Contributor Author

fPolic commented May 27, 2024

The grid gets laggy when you have many variants...

I addressed the other comment but I think that we should address bulk editor WIP stuff and performance optimisations in a separate ticket

Copy link
Contributor

@olivermrbl olivermrbl left a comment

Choose a reason for hiding this comment

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

Approving this, so we can tackle the rest in follow-up PRs. What do you think @kasperkristensen?

Copy link
Contributor

@kasperkristensen kasperkristensen left a comment

Choose a reason for hiding this comment

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

Lets merge it in, I think most if not all current issues are related to the DataGrid, which we can tackle later.

@olivermrbl olivermrbl merged commit 6117af2 into develop May 28, 2024
17 checks passed
@olivermrbl olivermrbl deleted the feat/product-create-reorg-v1 branch May 28, 2024 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants