Brand detail page styling #62

JiangLong2019 · 2023-09-24T08:41:48Z

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):

Fix #30

Test URLs:

Original: https://www.sunstar.com/brands/consumer-health-beauty/
Before: https://main--sunstar--hlxsites.hlx.live/_drafts/jiang/brands/consumer-health-beauty
After: https://issue-30--sunstar--hlxsites.hlx.live/brands/consumer-health-beauty
If you are adding a new block or a variation to an existing block, please fill below:

Block library path: https://--sunstar--hlxsites.hlx.page/tools/sidekick/library.html?plugin=blocks&path=/sidekick/blocks/&index=0

aem-code-sync · 2023-09-24T08:41:50Z

Hello, I'm Franklin Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

Re-run PSI Checks

aem-code-sync · 2023-09-24T08:41:53Z

Page	Scores	Audits	Google
/_drafts/jiang/brands/consumer-health-beauty		Lighthouse returned error: Something went wrong.

aem-code-sync · 2023-09-24T08:42:51Z

Page	Scores	Audits	Google
/_drafts/jiang/brands/consumer-health-beauty

bosschaert · 2023-09-24T20:20:29Z

styles/styles.css

@@ -432,6 +432,57 @@ main .section.full-width>.section-container {
  padding-right: 0;
 }

+/* START section - brand detail page */


I'm wondering, if these sections, which you neatly marked with START/END only apply to the brands page, would it make sense to put them in a page-specific style? I know it was said that we'd like to maybe avoid page-specific styles but maybe it's better than putting styles that apply to only 1 page in the general styles.css?

@bosschaert Yeah, it's pretty page-level styling. I had the same question, but as we discussed in the standup meeting, the page-level style won't be used in sunstar.com site.
@sdmcraft Shall we revive page-level styling?

Can't we use general styles in place of page level styles. I think we agreed over one approach that if we encounter such kind of cases, we will ask Dejan regarding that case.

I was just wondering how far does narrow section style take us on this. Here's what I see. Looks somewhat similar to live but not quite there. I think the images should be centered and margin/paddings should be adjusted. But likely the narrow section variant itself can benefit from these if we add these styles to it.

I just tried to use narrow but it doesn't look perfect to me. To reproduce the original site, if I add the styling to narrow, I think it will impact other pages such as image styling. Any suggestion in this case? For now, I added it separately into style.css.

narrow version:
https://issue-30--sunstar--hlxsites.hlx.page/_drafts/jiang/brands/consumer-health-beauty-withnarrower

Non-narrow version:
https://issue-30--sunstar--hlxsites.hlx.page/_drafts/jiang/brands/consumer-health-beauty

I see that you use the narrower style right now, the differences I personally see are:

The kendojo and equitance logo images are not centred and a bit bigger, I think that you can add two variants to the image collage, something like zoomed-in and centred to achieve the live website size and positioning

The rest of the images are smaller; I think you can add a section style called something like narrow-pictures that sets the img widths

The text is centred, not left-aligned; I think another section style called something like left-aligned-text would do the trick; either this or a variation of the text block that does the same

Headings are smaller; another section style called something like bigger-headings

Just my two cents

Thanks @dnbute! It's an approach that I thought of before but I'd like to reduce the authoring effort by avoiding adding more variants to blocks, that's why I put all staff into one metadata.

It looks like a maze game, @sdmcraft @bosschaert @jindaliiita any thoughts?

@sdmcraft @bosschaert @jindaliiita @dnbute Okay, I moved out the styling from styles.css and added them to blocks. Please help to review.
https://issue-30--sunstar--hlxsites.hlx.page/_drafts/jiang/brands/consumer-health-beauty

dnbute · 2023-09-25T10:00:22Z

styles/styles.css

+  flex: unset;
+}
+
+main .brand-detail-page p:has(img) {


I think Firefox doesn't support :has out of the box
https://caniuse.com/?search=has

Very good point 👍

@dnbute thanks, this has been fixed

aem-code-sync · 2023-10-04T07:15:44Z

Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

Re-run PSI Checks

aem-code-sync · 2023-10-04T07:16:00Z

Page	Scores	Audits	Google
/_drafts/jiang/brands/consumer-health-beauty

github-actions · 2023-10-04T07:17:14Z

🔸 1 visual difference detected

/career/yuya-yoshisue (main vs branch)
Expected an image 1280px by 3312px, received 1280px by 2819px. 2036368 pixels (ratio 0.49 of all image pixels) are different.

The diff images are attached in the artifact

aem-code-sync · 2023-10-04T07:31:50Z

Page	Scores	Audits	Google
/_drafts/jiang/brands/consumer-health-beauty

github-actions · 2023-10-04T07:33:16Z

🔸 1 visual difference detected

/career/yuya-yoshisue (main vs branch)
Expected an image 1280px by 3312px, received 1280px by 2819px. 2036368 pixels (ratio 0.49 of all image pixels) are different.

The diff images are attached in the artifact

aem-code-sync · 2023-10-05T03:48:07Z

Page	Scores	Audits	Google
/_drafts/jiang/brands/consumer-health-beauty

github-actions · 2023-10-05T03:50:14Z

🔸 9 visual differences detected

/career/yuya-yoshisue (main vs branch)
Expected an image 1280px by 3933px, received 1280px by 3981px. 325689 pixels (ratio 0.07 of all image pixels) are different.
/sidekick/blocks/collage (main vs branch)
Expected an image 1280px by 5160px, received 1280px by 5206px.
/sidekick/blocks/hero (main vs branch)
Expected an image 1441px by 4004px, received 1441px by 4052px.
/sidekick/blocks/columns (main vs branch)
Expected an image 1280px by 7092px, received 1280px by 7070px.
/sidekick/blocks/cards (main vs branch)
Expected an image 1280px by 1011px, received 1280px by 1155px. 39706 pixels (ratio 0.03 of all image pixels) are different.
/sidekick/blocks/text-image (main vs branch)
Expected an image 1280px by 4107px, received 1280px by 4108px.
/sidekick/blocks/carousel (main vs branch)
Expected an image 1280px by 1306px, received 1280px by 1354px. 146329 pixels (ratio 0.09 of all image pixels) are different.
/sidekick/blocks/sections (main vs branch)
Expected an image 1312px by 6512px, received 1312px by 6544px. 270901 pixels (ratio 0.04 of all image pixels) are different.
/sidekick/blocks/career-apply (main vs branch)
40960 pixels (ratio 0.05 of all image pixels) are different.

The diff images are attached in the artifact

aem-code-sync · 2023-10-05T03:50:26Z

Page	Scores	Audits	Google
/_drafts/jiang/brands/consumer-health-beauty

github-actions · 2023-10-05T03:52:26Z

🔸 9 visual differences detected

/career/yuya-yoshisue (main vs branch)
Expected an image 1280px by 3933px, received 1280px by 3981px. 325689 pixels (ratio 0.07 of all image pixels) are different.
/sidekick/blocks/hero (main vs branch)
Expected an image 1441px by 4004px, received 1441px by 4052px.
/sidekick/blocks/collage (main vs branch)
Expected an image 1280px by 5160px, received 1280px by 5206px.
/sidekick/blocks/columns (main vs branch)
Expected an image 1280px by 7092px, received 1280px by 7070px.
/sidekick/blocks/cards (main vs branch)
Expected an image 1280px by 1011px, received 1280px by 1155px. 39706 pixels (ratio 0.03 of all image pixels) are different.
/sidekick/blocks/text-image (main vs branch)
Expected an image 1280px by 4107px, received 1280px by 4108px.
/sidekick/blocks/carousel (main vs branch)
Expected an image 1280px by 1306px, received 1280px by 1354px. 146329 pixels (ratio 0.09 of all image pixels) are different.
/sidekick/blocks/sections (main vs branch)
Expected an image 1312px by 6512px, received 1312px by 6544px. 270901 pixels (ratio 0.04 of all image pixels) are different.
/sidekick/blocks/career-apply (main vs branch)
40960 pixels (ratio 0.05 of all image pixels) are different.

The diff images are attached in the artifact

sdmcraft · 2023-10-05T05:41:18Z

Please add a brand detail page here for it to be included in visual tests.

aem-code-sync · 2023-10-05T05:45:15Z

Page	Scores	Audits	Google
/_drafts/jiang/brands/consumer-health-beauty

aem-code-sync · 2023-10-05T05:59:26Z

Page	Scores	Audits	Google
/_drafts/jiang/brands/consumer-health-beauty

github-actions · 2023-10-05T06:12:46Z

🔸 1 visual difference detected

/brands/consumer-health-beauty (main vs branch)
Expected an image 1280px by 4426px, received 1280px by 4038px. 1325261 pixels (ratio 0.24 of all image pixels) are different.

The diff images are attached in the artifact

sdmcraft

Since you are adding new variations for the collage block, please add examples for it in the block library. Also please review the detected visual differences before merging to validate that they are all expected.

JiangLong2019 added 2 commits September 20, 2023 15:40

Brand detail page styling ver1

b146c35

Merge branch 'main' into issue-30

77a17a3

JiangLong2019 requested review from bosschaert, sdmcraft and jindaliiita September 24, 2023 08:41

bosschaert reviewed Sep 24, 2023

View reviewed changes

dnbute reviewed Sep 25, 2023

View reviewed changes

sdmcraft added this to the Milestone-2 milestone Sep 25, 2023

Merge branch 'main' into issue-30

dd23fdd

Bug fix

4576645

Refactoring

08b6452

Fix code check

500c306

JiangLong2019 modified the milestones: Milestone-2, Milestone-3 Oct 5, 2023

Merge branch 'main' into issue-30

4dcd8d1

Add visual test

5285b15

sdmcraft approved these changes Oct 5, 2023

View reviewed changes

JiangLong2019 merged commit 15299dc into main Oct 5, 2023
4 checks passed

JiangLong2019 deleted the issue-30 branch October 5, 2023 06:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Brand detail page styling #62

Brand detail page styling #62

JiangLong2019 commented Sep 24, 2023 •

edited

Loading

aem-code-sync bot commented Sep 24, 2023

aem-code-sync bot commented Sep 24, 2023

aem-code-sync bot commented Sep 24, 2023

bosschaert Sep 24, 2023

JiangLong2019 Sep 25, 2023

jindaliiita Sep 25, 2023

sdmcraft Sep 25, 2023 •

edited

Loading

JiangLong2019 Oct 4, 2023 •

edited

Loading

dnbute Oct 4, 2023 •

edited

Loading

JiangLong2019 Oct 4, 2023

JiangLong2019 Oct 5, 2023

dnbute Sep 25, 2023

jindaliiita Sep 25, 2023

JiangLong2019 Oct 4, 2023

aem-code-sync bot commented Oct 4, 2023

aem-code-sync bot commented Oct 4, 2023

github-actions bot commented Oct 4, 2023

aem-code-sync bot commented Oct 4, 2023

github-actions bot commented Oct 4, 2023

aem-code-sync bot commented Oct 5, 2023

github-actions bot commented Oct 5, 2023

aem-code-sync bot commented Oct 5, 2023

github-actions bot commented Oct 5, 2023

sdmcraft commented Oct 5, 2023

aem-code-sync bot commented Oct 5, 2023

aem-code-sync bot commented Oct 5, 2023

github-actions bot commented Oct 5, 2023

sdmcraft left a comment

Brand detail page styling #62

Brand detail page styling #62

Conversation

JiangLong2019 commented Sep 24, 2023 • edited Loading

aem-code-sync bot commented Sep 24, 2023

aem-code-sync bot commented Sep 24, 2023

aem-code-sync bot commented Sep 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdmcraft Sep 25, 2023 • edited Loading

Choose a reason for hiding this comment

JiangLong2019 Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

dnbute Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aem-code-sync bot commented Oct 4, 2023

aem-code-sync bot commented Oct 4, 2023

github-actions bot commented Oct 4, 2023

🔸 1 visual difference detected

aem-code-sync bot commented Oct 4, 2023

github-actions bot commented Oct 4, 2023

🔸 1 visual difference detected

aem-code-sync bot commented Oct 5, 2023

github-actions bot commented Oct 5, 2023

🔸 9 visual differences detected

aem-code-sync bot commented Oct 5, 2023

github-actions bot commented Oct 5, 2023

🔸 9 visual differences detected

sdmcraft commented Oct 5, 2023

aem-code-sync bot commented Oct 5, 2023

aem-code-sync bot commented Oct 5, 2023

github-actions bot commented Oct 5, 2023

🔸 1 visual difference detected

sdmcraft left a comment

Choose a reason for hiding this comment

JiangLong2019 commented Sep 24, 2023 •

edited

Loading

sdmcraft Sep 25, 2023 •

edited

Loading

JiangLong2019 Oct 4, 2023 •

edited

Loading

dnbute Oct 4, 2023 •

edited

Loading