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

feat(seo): optimized category URLs with full category path #1163

Merged
merged 1 commit into from
Jul 19, 2022

Conversation

Eisie96
Copy link
Contributor

@Eisie96 Eisie96 commented May 17, 2022

PR Type

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ x ] Refactoring (no functional changes, no API changes)
[ ] Build-related changes
[ ] CI-related changes
[ ] Documentation content changes
[ ] Application / infrastructure changes
[ ] Other:

What Is the Behavior?

The current category url contains only the current category displayname with an appended categoryUniqueId. (/Acer-Sub-catComputers.897.897_Acer.Acer-Sub)

This needs to be improved for SEO. Every category page should contain the whole path from the top level to the requested category. Only display-names should be used. The needed categoryUniqueId to get the data from the ICM should be at the end of the url. (/acer/sub/acer-sub-catComputers.897.897_Acer.Acer-Sub)

Does this PR Introduce a Breaking Change?

[x] Yes

Other Information

AB#76756

@Eisie96 Eisie96 marked this pull request as ready for review May 18, 2022 09:46
@MaxKless MaxKless self-requested a review May 18, 2022 11:05
src/app/core/routing/category/category.route.ts Outdated Show resolved Hide resolved
src/app/extensions/seo/store/seo/seo.effects.ts Outdated Show resolved Hide resolved
src/app/core/routing/category/category-route.pipe.ts Outdated Show resolved Hide resolved
@dhhyi dhhyi self-requested a review May 23, 2022 12:29
dhhyi
dhhyi previously requested changes May 23, 2022
Copy link
Collaborator

@dhhyi dhhyi left a comment

Choose a reason for hiding this comment

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

@Eisie96 Is there anything that speaks against including the path categories into the CategoryView model? That way you wouldn't need an additional store selector and additional arguments to the functions... 🤔

EDIT 1: I originally removed that in #284 as part of a memoization optimization (getting rid of the functions), and I think the pathCategories were no longer required as CategoryView at that time, but at the moment I see no reason to re-add them like that in a properly memoized way.

EDIT 2: With the extension of the view, you can also simplify things in #1164

@Eisie96
Copy link
Contributor Author

Eisie96 commented May 23, 2022

@dhhyi Thank you for your review. It is a really good idea to put the category path to the category view! The CategoryRoutePipe is now a pure function. One thing i have to change would be the getCategory(uniqueId) selector. It generates the category view with the subtree of the given uniqueid. But the whole tree is necessary to get all elements from the category path.

@dhhyi dhhyi dismissed their stale review May 25, 2022 10:13

Suggestion implemented 👍

@Eisie96 Eisie96 force-pushed the refactor/improve-category-url branch from 2ca51c2 to 86d7e5d Compare May 25, 2022 13:58
shauke
shauke previously approved these changes May 30, 2022
src/app/core/routing/category/category-route.pipe.ts Outdated Show resolved Hide resolved
@shauke shauke added this to the 3.0 milestone May 31, 2022
MaxKless
MaxKless previously approved these changes May 31, 2022
@Eisie96 Eisie96 dismissed stale reviews from MaxKless and shauke via af5777f June 1, 2022 07:53
@Eisie96 Eisie96 force-pushed the refactor/improve-category-url branch 2 times, most recently from 55076e1 to 9f9b456 Compare June 1, 2022 11:42
@shauke shauke force-pushed the refactor/improve-category-url branch 4 times, most recently from 96e1879 to 7367d1e Compare June 15, 2022 14:03
shauke pushed a commit that referenced this pull request Jun 15, 2022
* changed category id marker from 'cat' to 'ctg'
* unified base for SEO route generation
@shauke shauke force-pushed the refactor/improve-category-url branch from 7367d1e to dc6f236 Compare June 15, 2022 19:40
@shauke shauke changed the title refactor: improve category url feat(seo): optimized category URLs with full category path Jun 15, 2022
* changed category id marker from 'cat' to 'ctg'
* unified base for SEO route generation

BREAKING CHANGES: Changed category routes/URLs (see [Migrations / 2.4 to 3.0](https://github.com/intershop/intershop-pwa/blob/develop/docs/guides/migrations.md#24-to-30) for more details).
@shauke shauke force-pushed the refactor/improve-category-url branch from dc6f236 to bd25b16 Compare June 15, 2022 20:26
@shauke shauke merged commit 2a212bb into develop Jul 19, 2022
@shauke shauke deleted the refactor/improve-category-url branch July 19, 2022 14:54
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.

4 participants