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.

Sign up for GitHub

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

Jump to bottom

fix(api): PDP and PLP breadcrumb list item's URL paths #1148

Merged
merged 2 commits into from
Feb 15, 2022
Merged

fix(api): PDP and PLP breadcrumb list item's URL paths #1148

merged 2 commits into from
Feb 15, 2022

Conversation

filipewl
Copy link
Contributor

@filipewl filipewl commented Feb 10, 2022 •
edited
Loading

What's the purpose of this pull request?

  • Fix upper case in the PLP/collection's breadcrumb.
Before After
/Kitchen-and-Home-Appliances (not working) /kitchen-and-home-appliances (working)
Screen Shot 2022-02-10 at 23 03 45 Screen Shot 2022-02-10 at 23 07 23
  • Fix spaces in the PDP/product's breadcrumb.
Before After
/kitchen and home appliances (not working) /kitchen-and-home-appliances (working)
Screen Shot 2022-02-10 at 23 03 00 Screen Shot 2022-02-10 at 23 14 01

base.store Deploy Preview

8d954a56253469514b1ffe7d2d8de29399db9db0

@netlify
Copy link

netlify bot commented Feb 10, 2022 •
edited
Loading

✔️ Deploy Preview for faststoreui ready!

🔨 Explore the source changes: 6926eb9

🔍 Inspect the deploy log: https://app.netlify.com/sites/faststoreui/deploys/6205aabdf9532400078860bd

😎 Browse the preview: https://deploy-preview-1148--faststoreui.netlify.app

@netlify
Copy link

netlify bot commented Feb 10, 2022 •
edited
Loading

✔️ Deploy Preview for faststore-docs ready!

🔨 Explore the source changes: 6926eb9

🔍 Inspect the deploy log: https://app.netlify.com/sites/faststore-docs/deploys/6205aabd28f2f800070f184f

😎 Browse the preview: https://deploy-preview-1148--faststore-docs.netlify.app

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 11, 2022 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6926eb9:

Sandbox Source
Store UI Typescript Configuration

@filipewl filipewl changed the title Fix product breadcrumb list item's URL path fix(api): PDP and PLP breadcrumb list item's URL paths Feb 11, 2022
filipewl pushed a commit to vtex-sites/base.store that referenced this pull request Feb 11, 2022
fix: Breadcrumb links for sub-categories
5aa41af 
Together with vtex/faststore#1148, there's no need of this `buildUrl` function. This function was actually building the wrong path for sub-categories (e.g. `/desk` instead of `/office/desk`).
@filipewl filipewl self-assigned this Feb 11, 2022
@filipewl filipewl added the bug Something isn't working label Feb 11, 2022
@filipewl filipewl marked this pull request as ready for review February 11, 2022 10:24
@filipewl filipewl requested a review from a team as a code owner February 11, 2022 10:24
@igorbrasileiro
Copy link
Contributor

@tlgimenes the API is the right place to handle e normalize those kind of things, right?

@filipewl filipewl requested a review from tlgimenes February 11, 2022 14:12
eduardoformiga
eduardoformiga approved these changes Feb 11, 2022
View reviewed changes
Copy link
Member

@eduardoformiga eduardoformiga left a comment

Choose a reason for hiding this comment

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

good job! 👏

@filipewl filipewl merged commit ea1790a into master Feb 15, 2022
@filipewl filipewl deleted the fix/breadcrumb-item-path branch February 15, 2022 19:34
filipewl pushed a commit to vtex-sites/base.store that referenced this pull request Feb 15, 2022
fix(api): PDP and PLP breadcrumb list item's URL paths
f5dd08b 
Fix broken breadcrumb paths returned by `@faststore/api` that has been fixed in version [v1.5.8](https://github.com/vtex/faststore/releases/tag/v1.5.8). See vtex/faststore#1148 for more information.
@filipewl filipewl mentioned this pull request Feb 15, 2022
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tlgimenes tlgimenes tlgimenes approved these changes

@eduardoformiga eduardoformiga eduardoformiga approved these changes

@igorbrasileiro igorbrasileiro igorbrasileiro approved these changes

Assignees

@filipewl filipewl

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@filipewl @igorbrasileiro @tlgimenes @eduardoformiga