aemsites / stericycle-shared

Edge Delivery Service Site for Stericycle sites
https://www.shredit.com, https://www.stericycle.com
Apache License 2.0
0 stars 1 forks source link

WIP Add Shred-it’s 6-Step Security Protocol Section #74

Closed allanhero closed 3 months ago

allanhero commented 4 months ago

Fix #70

Test URLs:

Screenshot 2024-07-22 at 11 49 22 AM

aem-code-sync[bot] commented 4 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 * [f01129d](https://github.com/aemsites/stericycle-shared/commit/f01129de5da5b2709d0bf28edfa20cc3ff5d7683) :white_check_mark: (latest) * [7e2696e](https://github.com/aemsites/stericycle-shared/commit/7e2696e979bd30642c1d3bca7995ba0168ec4113) :white_check_mark: * [bb58b52](https://github.com/aemsites/stericycle-shared/commit/bb58b528d744ede15a1be899885bb5b7f656143c) :white_check_mark: * [d6986a5](https://github.com/aemsites/stericycle-shared/commit/d6986a5a77d1a87da0a39cb4e428490cda92cdda) :white_check_mark:
aem-code-sync[bot] commented 4 months ago
Page Scores Audits Google
/en-us/secure-shredding-services/specialty-shredding-service PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
kronnox commented 4 months ago

Please wait with merging this. I'll have a look at it later today; might have some additional feedback on this

kronnox commented 4 months ago

@allanhero I think we might have some overlap on what already has been implemented in #40 (https://main--stericycle-shared--aemsites.hlx.page/drafts/janlucaa/list-styles) some time ago. Sorry I didn't check the issues earlier and mentioned this at an earlier point in time.

Although, I took a different way of implementing this back then, than you did here. My implementation took the more generic route of adding a section style to change the appearance of lists. Even though this probably makes a lot of sense for the other list style with the orange triangles, it's probably of no benefit here, since the style with the big numbers is only used in this exact constellation (?). So I won't be the judge of what is the better implementation of this from an authoring standpoint.

Don't get me wrong, I'm fine with either way of solving this, I just don't want to have duplicate styles that basically are meant to achieve the same thing, so let's either decide on one implementation or "merge" them to go with the best of both

seanathero commented 4 months ago

@allanhero let's meet to discuss this. Thanks for calling attention to this @kronnox !

allanhero commented 4 months ago

@allanhero let's meet to discuss this. Thanks for calling attention to this @kronnox !

@seanathero The changes were made to adapt the decorator to @kronnox implementation of the list styles. Let me know if we need to fix something else.

allanhero commented 4 months ago

@seanathero @kronnox @dkuntze The link is modified to point the component to its original page.