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 missing swatches modal reappear #49

Open
wants to merge 4 commits into
base: next
Choose a base branch
from

Conversation

jtomaszewski
Copy link
Contributor

Currently it appears only once. If user closes the modal and clicks "Add to cart" again, the modal doesn't appear again.

Besides fixing that, I also did two fixes:

  • make the "overlay" clickable, so user can easily go back to product details screen
  • hide the error message - it isn't really needed, when the modal is shown anyways [when no required swatch is selected]

@msiewierska
Copy link
Contributor

msiewierska commented Feb 23, 2021

Hi @jtomaszewski Thank you a lot for your contribution.

modal appears only once It was done on purpose that the missing swatches modal appears only once. It seems to us that it is quite a corner case that a user does not check any swatch even on the modal. Error message stays in this case in the buybox. Do you think that showing modal more times could increase UX?

hide the error message - we cannot just hide error message for all cases because missing swatches modal is just an option - it is turned on by default but the case with standard error messages and no modal should also be supported.

Please rebase your PR and remove css part.

@jtomaszewski
Copy link
Contributor Author

Error message stays in this case in the buybox. Do you think that showing modal more times could increase UX?

I think it's better for it to be consistent (why would it react differently on the 2nd click)? error messages are not always that clearly visible to the user if they are seen after a page redirect in some buybox.

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.

2 participants