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

added condition for checkbox #3

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

AndreiAnton
Copy link

@AndreiAnton AndreiAnton commented Oct 6, 2020

Added a condition for the checkbox based on the category of the first selected for comparison product

Update: Added some CSS handles

@vtex-io-ci-cd
Copy link

vtex-io-ci-cd bot commented Oct 6, 2020

Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖

Please select which version do you want to release:

  • Patch (backwards-compatible bug fixes)

  • Minor (backwards-compatible functionality)

  • Major (incompatible API changes)

And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.

  • No thanks, I would rather do it manually 😞

@vtex-io-docs-bot
Copy link

Beep boop 🤖

I noticed you're not using the Docs Builder properly yet, if you need help to set that up please go to IO Documentation

@oviolion
Copy link
Contributor

@klzns would you mind taking a look over this when you've got the time? Thanks!

klzns
klzns previously requested changes Oct 27, 2020
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Breno Calazans <[email protected]>
@@ -68,11 +69,18 @@ const ProductSelector = ({ showToast, intl }: Props) => {
}
}

const categoryArrayLength = valuesFromContext?.product?.categories.length;
const categoryAvailable = categoryArrayLength === 1 ? valuesFromContext?.product?.categories[0] : valuesFromContext?.product?.categories[categoryArrayLength - 2]
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you choosing this particular category?

@alessandrasa
Copy link
Contributor

I am not sure that it is a good experience for the buyer having the checkbox disabled for no explained reason. I will check with @rsimoens what we could do to communicate it clearer.

@rsimoens
Copy link

I would like to know more about the motivation of this condition.

At first, I think this kind of limitation may not be necessary at all, and might lead to reduce store conversion if not configured right. E.g.: comparing DSLR cameras with Mirrorless cameras is important in the decision making process and, depending on how the store's categories are organised, with this condition it would not be possible.

From my point of view, the risk of users selecting products to compare with no mutual specification fields is not worth this new condition complexity on buyer's experience and store configuration.

Besides that, I agree with @alessandrasa, without proper feedback it would be hard for users to understand why they can't select some products to compare depending on how the store organised the category tree.

@rsimoens
Copy link

As example, on these sites I'm able as a buyer to compare products from completely different categories:

https://www.bestbuy.com/site/compare?skus=5044601,5712431
https://www.staples.com/compare?skus=24444863,868972
https://www.grainger.com/content/compare#compareSkus%3D5U254%2C3DYJ6

Buyers may using the comparison feature with different intents, and this condition might get in their way to make a purchase.

@klzns klzns dismissed their stale review October 29, 2020 20:40

pr changed

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.

6 participants