hlxsites / sunstar

AEM Franklin repo for sunstar.com
Apache License 2.0
1 stars 3 forks source link

Restructure tag page - change the logic of tag rendering #576

Closed JiangLong2019 closed 6 months ago

JiangLong2019 commented 7 months ago

Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after), along with a short summary of changes:

Issue

Fixes #575

Changelog:

  1. In feed block and breadcrumb, change the way of getting tag id identifier from url parameter to path. Ex, feed-tags=XXX -> /tag/XXX.
  2. In tag block, change the tag link format from url parameter to path. Ex, feed-tags=XXX -> /tag/XXX.
  3. Remove tag URL parameter redirection from 404.html
  4. Redirect existing tag page URL to the new format.

Test URLs:

aem-code-sync[bot] commented 7 months ago

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed. In case there are problems, just click a checkbox below to rerun the respective action.

Commits * [edbc456](https://github.com/hlxsites/sunstar/commit/edbc456a582eafa852585ab836950698b9b49777) :white_check_mark: (latest) * [3e6c6a8](https://github.com/hlxsites/sunstar/commit/3e6c6a872c8fc7ebe47ce10d3cb64599cc1e03ff) :white_check_mark:
aem-code-sync[bot] commented 7 months ago
Page Scores Audits Google
/healthy-thinking/tag/education PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
JiangLong2019 commented 7 months ago

@sdmcraft EDS doesn't support url parameters in redirect source URL, I'll try to find any other way to deal with it rather than redirects.xls

Redirect existing tag page URL to the new format.

https://discord.com/channels/1131492224371277874/1207624789083168768/1207749171969265664

jindaliiita commented 7 months ago

Changes looks fine to me

sdmcraft commented 7 months ago

@sdmcraft EDS doesn't support url parameters in redirect source URL, I'll try to find any other way to deal with it rather than redirects.xls

Redirect existing tag page URL to the new format.

https://discord.com/channels/1131492224371277874/1207624789083168768/1207749171969265664

If query parameter based redirects are not possible then how would we solve the original problem i.e. having the right canonical url on the page?

JiangLong2019 commented 7 months ago

@sdmcraft

If query parameter based redirects are not possible then how would we solve the original problem i.e. having the right canonical url on the page?

After the change, each tag page owns correct canonical url now. The problem is that we wanted to redirect current site tag page to the new tag page, let's say, Current tag page - https://www.sunstar.com/healthy-thinking/tag?feed-tags=germany New tag page - https://www.sunstar.com/healthy-thinking/tag/germany

In redirects.xls, the source URL doesn't support URL with parameter, we cannot use redirects.xls for the redirection. In my opinion, I don't think it's necessary, maybe we can ignore it.

JiangLong2019 commented 7 months ago

@sdmcraft As we discussed, I've added the redirection for URL parameter tag page. Please have a check. https://github.com/hlxsites/sunstar/pull/576/files#diff-fbceb58f32b99b8a5309251f8c49ac8eda82fba58b905c600c92bce1ed2a1c8c