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

[BUGFIX] Show a message when a category has no products assigned #1496

Merged

Conversation

Jordaneisenburger
Copy link
Member

@Jordaneisenburger Jordaneisenburger commented Jul 31, 2019

Description

This PR introduces a new component in RootComponents/Category called NoProductsFound which is rendered when a category has no products.

It also:

  • Includes a NoProductsFound talon in Peregrine
  • Removes packages/peregrine/lib/store/actions/catalog/mockData.js which wasn't being used and was out of date anyway (which caused confusion)

Related Issue

Closes #823.
Closes #1678

Verification Steps

  1. Create an empty category in M2 Admin by going to Catalog > Categories > Add Subcategory
  2. Do not assign any products to your new category
  3. In your browser's Application tab, click Clear Site Data and refresh the page
  4. Navigate to Venia's homepage
  5. See that your new empty category appears1
  6. Click the empty category
  7. See that you are presented with a friendly message and other categories are recommended to you2
  8. Verify that the links work correctly (take you to the correct category page)

Screenshots / Screen Captures (if appropriate)

1
Screen Shot 2019-09-30 at 10 45 30 AM

2
Screen Shot 2019-09-30 at 11 11 35 AM

Proposed Labels for Change Type/Package

  • major (e.g x.0.0 - a breaking change)
  • minor (e.g 0.x.0 - a backwards compatible addition)
  • patch (e.g 0.0.x - a bug fix)

Checklist:

  • I have updated the documentation accordingly, if necessary.
  • I have added tests to cover my changes, if necessary.

@magento-cicd2
Copy link

magento-cicd2 commented Jul 31, 2019

CLA assistant check
All committers have signed the CLA.

@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Jul 31, 2019

Messages
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).

Generated by 🚫 dangerJS against 481b28e

@supernova-at
Copy link
Contributor

hey @Jordaneisenburger haven't had a chance to look at this yet but was curious about the CLA agreement ... do you have the option to sign it again now that the repo is in the magento organization?

@Jordaneisenburger
Copy link
Member Author

@supernova-at CLA signed.

@supernova-at supernova-at self-assigned this Aug 6, 2019
Copy link
Contributor

@supernova-at supernova-at left a comment

Choose a reason for hiding this comment

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

Very cool, thanks for tackling this!

I've added the needs-ux label to this because I don't think we have any mockups for this scenario.

For consistency I'm thinking we'd want it to look more like other "error" cases like this (but probably not to say "Closed"):

Screen Shot 2019-08-06 at 10 29 54 AM

https://magento.invisionapp.com/share/WQN5F7BYBPG#/screens/329211515

align-items: center;
display: flex;
flex-direction: column;
height: 100vh;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want this to be 100%?

@supernova-at
Copy link
Contributor

Looks like there is a merge conflict to resolve as well.

@supernova-at supernova-at added needs-ux Collaboration and review from UX team is required version: Patch This changeset includes backwards compatible bug fixes. labels Aug 6, 2019
@Jordaneisenburger
Copy link
Member Author

Yes agreed that the UX/UI needs to be more consistent with the rest of venia, @soumya-ashok what do you think would be best here ?

@soumya-ashok
Copy link

soumya-ashok commented Aug 7, 2019

@Jordaneisenburger Here are a couple of options

No products found


No products found 2

Based on what is possible with GraphQL, it might be good to provide direction to the shopper on this error screen to either browse other categories, related or similar products to what they were trying to view. This would mitigate the effect of a dead-end as far as a shopping experience is concerned.

@supernova-at @awilcoxa Thoughts?

@brendanfalkowski
Copy link
Contributor

@soumya-ashok — Good improvement over the "closed" sign :)

For larger stores that have automated catalog imports, this scenario probably never occurs because if a category were empty it would be disabled until the next catalog refresh.

For smaller stores, they sometimes have trouble managing the list of categories or products to show in rare contexts like this (out of sight, out of mind) so I've found the best default approach is simply rendering the top-level of categories. That redirects an unknown search term into the broadest taxonomy of the site.

Then if the store wants more control we might create a hidden category of "best seller" products to render in various empty states in the application so they always see something.

@soumya-ashok
Copy link

Thanks @brendanfalkowski ! :) I did bring up here with the team that this seemed like an edge-case. I see what you're saying about the broad list of categories and the "best seller" approach as well.

If I were to pick, I would go the "best seller" or product recommendations route because this listing view is something we would already have an established pattern for elsewhere on the site and the fact that engaging with one of those to go into the PDP is one-step closer to conversion than a category page.

@awilcoxa
Copy link

awilcoxa commented Aug 7, 2019

Agree with Soumya here, would need some weigh in from the team on LOE in any case since we would want to avoid any large effort for such a small (potentially) edge case.

@supernova-at
Copy link
Contributor

would need some weigh in from the team on LOE

The lowest level-of-effort would be to link to some or all of the top-level categories, since we already have that data handy.

@supernova-at
Copy link
Contributor

@Jordaneisenburger , @awilcoxa is out this week - I'm confident we can go ahead and link some top-level categories on Soumya's mockup and this will be good to go.

Thanks again!

@Jordaneisenburger
Copy link
Member Author

@Jordaneisenburger , @awilcoxa is out this week - I'm confident we can go ahead and link some top-level categories on Soumya's mockup and this will be good to go.

Thanks again!

Sweet, will pick this one up again

@supernova-at supernova-at removed their assignment Aug 14, 2019
Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

A few comments otherwise this is great.

sirugh
sirugh previously approved these changes Oct 1, 2019
@dpatil-magento
Copy link
Contributor

dpatil-magento commented Oct 2, 2019

@supernova-at I don't see suggested category list. Is this expected?

image

@supernova-at
Copy link
Contributor

I don't see suggested category list. Is this expected?

That is not expected, but the categories do take some time to appear if you start directly on that page.

@magento magento deleted a comment from vercel bot Oct 2, 2019
@supernova-at
Copy link
Contributor

@dpatil-magento good catch! Fixed in f6fce9c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:peregrine pkg:pwa-buildpack pkg:venia-concept pkg:venia-ui version: Minor This changeset includes functionality added in a backwards compatible manner.
Projects
None yet
10 participants