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

Make Products Changeset Aware #1142

Closed
wants to merge 15 commits into from
Closed

Conversation

elisabeth-sorrell
Copy link
Collaborator

@elisabeth-sorrell elisabeth-sorrell commented Jul 8, 2022

@elisabeth-sorrell
Copy link
Collaborator Author

Screen Shot 2022-07-08 at 11 01 54 AM

@elisabeth-sorrell
Copy link
Collaborator Author

Screen Shot 2022-07-08 at 11 02 29 AM

@elisabeth-sorrell
Copy link
Collaborator Author

  • Having a little styling problems with the badge outline not stretching to the icon when a prop is changed...
  • Not sure if this is part of the scope, but the languageEngagement where the products are added to doesn't have a changeset badge with changed mode.
  • Also not sure what to do with the title for a Product with Other type. Should that also have a changeset badge? should it be even changeset aware in the first place?

Also note that I put icons on the right instead of the left. I thought it went better with the flow of how the props are displayed, instead of changing margins on a changeset (the icon overlaps the label when it's on the left.)

Copy link
Member

@CarsonF CarsonF left a comment

Choose a reason for hiding this comment

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

Looking good!

Will you rebase this on develop? I think you'll just have some MUI & React imports to adjust.

I need to study the ChangesetPropList component more.

src/common/fragments/changeset.graphql Outdated Show resolved Hide resolved
src/components/Changeset/ChangesetBadge.tsx Outdated Show resolved Hide resolved
src/scenes/Products/Create/CreateProduct.tsx Outdated Show resolved Hide resolved
src/components/Changeset/ChangesetBadge.tsx Outdated Show resolved Hide resolved
@elisabeth-sorrell elisabeth-sorrell force-pushed the addChangesetToProducts branch 2 times, most recently from 4ac1b2c to cf2760a Compare August 16, 2022 16:47
…uts of create and edit to have the correct variables for the CreateInput
…ngesetId) if it was created in a changeset, but route to just the engagement id if not in a changeset
…e you can control what side of the wrapped component the icon goes on, with the default being the left side of the component
…with a ChangesetBade with mode depending on whether that item is added or removed, rendering each prop list Item according to the render function of the param
…function wraps the value with whatever the 'parent' component passes in. Have the simple props in the ProductDetail wrap the value with a ChangesetPropertyBadge, and use the new ChangesetPropertyList for the list props
@sethmcknight
Copy link
Member

This lived to the end of its lifetime and died

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.

3 participants