[feature]: product page breadcrumbs #1960

sirugh · 2019-11-05T16:09:31Z

Description

Based on #1949. This PR adds breadcrumbs to the product page. Main differences:

Unlike the category page breadcrumbs, the last category text in the list is a link and will be underlined/clickable.
If a product has only one related category it will display a single breadcrumb to that category
~~If a product has multiple categories it will display a dropdown with multiple options.~~

Related Issue

Closes PWA-99.

Acceptance

Verification Stakeholders

@jimbo @schensley

Should we even display PDP breadcrumbs on mobile?
Is a dropdown the right UX?

Specification

Verification Steps

Go to a product with only one related category (isadora skirt). It should show regular breadcrumbs like on the category page.
Go to a product with multiple categories (valeria two-layer tank). It should also show regular breadcrumbs.

Screenshots / Screen Captures (if appropriate)

Checklist

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

PWAStudioBot · 2019-11-05T16:19:43Z

	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).
📖	Associated JIRA tickets: PWA-99.

Generated by 🚫 dangerJS against 965d313

sirugh · 2019-11-12T21:08:41Z

I just pushed some big changes to this PR. Rather than have a dropdown for breadcrumbs when there were multiple possible categories on a PDP I now:

Request categories for the product
Filter down the category breadcrumbs to a list of only leaf categories since the API gives us all the intermediate categories.
Select the first category id as the breadcrumb category for the product.

Note that the method of selection of which category breadcrumbs to display can cause a jarring display if a user navigated to the PDP by one route and are then shown a different breadcrumb set. See the gifs in the description.

jimbo · 2019-11-12T21:29:45Z

packages/peregrine/lib/talons/ProductFullDetail/useProductFullDetail.js

+
+    // Return the first category id of the potential leaf categories.
+    // This can potentially be jarring if the user navigated to a product and
+    // the breadcrumbs don't represent that navigation path.


I don't think it'll be jarring at all. Breadcrumbs should be a canonical path to the product as understood by the server (or in this case, estimated by the client using a simple heuristic: first category wins). We don't need to be in the business of showing the user their navigation history. That's what browser history and browser chrome are for.

At any rate, I like this a lot more than the dropdown.

Well I certainly don't want to lead opinion through this comment -- I can remove it and simply say "Until we can get the single canonical breadcrumb path to a product we will just select the first leaf category"

sirugh · 2019-11-12T22:52:53Z

packages/venia-ui/lib/fragments/productDetails.graphql

@@ -1,5 +1,11 @@
 fragment ProductDetails on ProductInterface {
    __typename
+    categories {


As an FYI we should add a comment in here at the top saying that updates to this fragment should be mirrored in getProductDetail and vice versa.

sirugh · 2019-11-12T23:24:51Z

packages/venia-ui/lib/components/ProductFullDetail/productFullDetail.js

+            <Breadcrumbs
+                categoryId={breadcrumbCategoryId}
+                currentProduct={productDetails.name}
+            />


This and the returned prop are the only changes. Everything just shifted cause of the <Fragment>.

tjwiebell

Looks great, have some minor suggestions for optimization and readability.

packages/peregrine/lib/talons/Breadcrumbs/useBreadcrumbs.js

packages/peregrine/lib/talons/ProductFullDetail/useProductFullDetail.js

tjwiebell

Hansel and Gretel would be proud, nice work 🍞 🍞 🍞

…n fetched

tjwiebell

See #1960 (review)

sirugh changed the base branch from develop to rugh/breadcrumbs November 5, 2019 16:12

PWAStudioBot added the pkg:venia-ui label Nov 5, 2019

sirugh added Don't Merge version: Minor This changeset includes functionality added in a backwards compatible manner. labels Nov 5, 2019

PWAStudioBot added the pkg:peregrine label Nov 5, 2019

sirugh changed the base branch from rugh/breadcrumbs to develop November 12, 2019 20:17

sirugh changed the base branch from develop to rugh/breadcrumbs November 12, 2019 20:17

Display breadcrumbs on the product page

73f994c

sirugh force-pushed the rugh/product-breadcrumbs branch from 18b3479 to 73f994c Compare November 12, 2019 21:04

sirugh changed the base branch from rugh/breadcrumbs to develop November 12, 2019 21:04

safety check for array ref

c4d65c9

jimbo reviewed Nov 12, 2019

View reviewed changes

m2-community-project bot assigned jimbo Nov 12, 2019

m2-community-project bot added the Progress: review label Nov 12, 2019

sirugh added 3 commits November 12, 2019 15:35

Display current product text if passed as prop

6c4fde3

Adds a comment

bd092c2

Adds a prop type

f504db6

sirugh force-pushed the rugh/product-breadcrumbs branch from 17046a7 to f504db6 Compare November 12, 2019 21:42

m2-community-project bot assigned sirugh Nov 12, 2019

Update comments

a13fbe2

sirugh force-pushed the rugh/product-breadcrumbs branch from ffd13b4 to a13fbe2 Compare November 12, 2019 21:58

sirugh removed the Don't Merge label Nov 12, 2019

sirugh commented Nov 12, 2019

View reviewed changes

Add comment to graphql query and fragment

9943a9f

tjwiebell suggested changes Nov 13, 2019

View reviewed changes

packages/peregrine/lib/talons/Breadcrumbs/useBreadcrumbs.js Show resolved Hide resolved

packages/peregrine/lib/talons/Breadcrumbs/useBreadcrumbs.js Outdated Show resolved Hide resolved

packages/peregrine/lib/talons/ProductFullDetail/useProductFullDetail.js Show resolved Hide resolved

m2-community-project bot assigned tjwiebell Nov 13, 2019

m2-community-project bot added Progress: changes requested and removed Progress: review labels Nov 13, 2019

sirugh and others added 3 commits November 14, 2019 09:22

Merge branch 'develop' into rugh/product-breadcrumbs

909a3e2

Address feedback

af134e6

Safely render breadcrumbs after checking potential fallback category id

89e1d70

tjwiebell previously approved these changes Nov 14, 2019

View reviewed changes

Remove default suffix but add fallback path for when data has not bee…

dedda65

…n fetched

sirugh dismissed tjwiebell’s stale review via dedda65 November 14, 2019 16:22

sirugh added 2 commits November 14, 2019 10:25

Add fallback for when data has not loaded or doesn't contain suffix

d7dfee9

add fallback for error state

1c85c0d

tjwiebell approved these changes Nov 14, 2019

View reviewed changes

Merge branch 'develop' into rugh/product-breadcrumbs

965d313

dpatil-magento merged commit 1c5a422 into develop Nov 14, 2019

dpatil-magento deleted the rugh/product-breadcrumbs branch November 14, 2019 19:24

m2-community-project bot removed the Progress: changes requested label Nov 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature]: product page breadcrumbs #1960

[feature]: product page breadcrumbs #1960

sirugh commented Nov 5, 2019 •

edited

Loading

PWAStudioBot commented Nov 5, 2019 •

edited

Loading

sirugh commented Nov 12, 2019 •

edited

Loading

jimbo Nov 12, 2019

sirugh Nov 12, 2019

sirugh Nov 12, 2019

sirugh Nov 12, 2019

tjwiebell left a comment

tjwiebell left a comment

tjwiebell left a comment

[feature]: product page breadcrumbs #1960

[feature]: product page breadcrumbs #1960

Conversation

sirugh commented Nov 5, 2019 • edited Loading

Description

Related Issue

Acceptance

Verification Stakeholders

Specification

Verification Steps

Screenshots / Screen Captures (if appropriate)

Checklist

PWAStudioBot commented Nov 5, 2019 • edited Loading

sirugh commented Nov 12, 2019 • edited Loading

jimbo Nov 12, 2019

Choose a reason for hiding this comment

sirugh Nov 12, 2019

Choose a reason for hiding this comment

sirugh Nov 12, 2019

Choose a reason for hiding this comment

sirugh Nov 12, 2019

Choose a reason for hiding this comment

tjwiebell left a comment

Choose a reason for hiding this comment

tjwiebell left a comment

Choose a reason for hiding this comment

tjwiebell left a comment

Choose a reason for hiding this comment

sirugh commented Nov 5, 2019 •

edited

Loading

PWAStudioBot commented Nov 5, 2019 •

edited

Loading

sirugh commented Nov 12, 2019 •

edited

Loading