Skip to content

MWPW-178300: pre-reserve wrapper heights to eliminate CLS on login page and modals - #525

Open
saurabhsircar11 wants to merge 23 commits into
stagefrom
MWPW-178300-susi-light-height
Open

MWPW-178300: pre-reserve wrapper heights to eliminate CLS on login page and modals#525
saurabhsircar11 wants to merge 23 commits into
stagefrom
MWPW-178300-susi-light-height

Conversation

@saurabhsircar11

@saurabhsircar11 saurabhsircar11 commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

Pre-reserves susi-light heights synchronously in init() before the async susi-sentry-light CDN component hydrates, reducing Cumulative Layout Shift (CLS) on the login page and in SUSI modals.

Tabs (login page only — prod susi-light tabs is authored on /express/login + locales):

  • applyTabsReserveFromAuthoring() sets --susi-tabs-panel-height to the tallest tab panel (max(standard, edu-express) = 521px, measured on login draft with the Students footer bubbles).
  • .susi-tab-panels uses min-height only (no fixed height / inner scroll) — same strategy as modals. Page scrolls if content exceeds the floor.
  • Tab switch: 0px layout delta verified on /drafts/sircar/login (Individual ↔ Students).

Modals & in-page variants:

  • applyModalWrapperReserve() sets --susi-modal-wrapper-height from variant class + client_id + layout flags (data-susi-wrapper-profile for debugging).
  • Modal .susi-wrapper uses min-height only — Milo .dialog-modal scrolls on short viewports (no inner wrapper scrollbar).

Measured wrapper floors (rebaselined from main/stage settled heights, no buffer): b2b-default 484, b2b-hed 291, b2b-email-first 409, edu-default 484, edu-hed 333, edu-business 393, legacy-edu-express 545, legacy-edu-hed 477, simplified 400, etc.

A11y: Inactive tab panels get aria-hidden + inert alongside .hide.

Tradeoff: Floors are set to hydrate-peak / tallest-tab heights. Where settled content is shorter, you may see empty space below the form — intentional CLS mitigation. Tokens were tightened vs earlier commits to reduce dead space while still preventing shift.


Jira Ticket

Resolves: MWPW-178300


Test URLs

Env URL
Before (stage) https://main--da-express-milo--adobecom.aem.live/drafts/sircar/susi-light-copy
After (modal QA) https://MWPW-178300-susi-light-height--da-express-milo--adobecom.aem.live/drafts/sircar/susi-light-copy?martech=off
After (modal QA — copy 2) https://MWPW-178300-susi-light-height--da-express-milo--adobecom.aem.live/drafts/sircar/susi-light-copy-copy?martech=off
After (login tabs) https://MWPW-178300-susi-light-height--da-express-milo--adobecom.aem.live/drafts/sircar/login?martech=off

Verification Steps

Tabs variant (login page):

  1. Open After (login tabs) in Chrome
  2. DevTools → Performance → record reload — expect minimal CLS from SUSI hydrate
  3. Switch Individual/Business ↔ Students/Teachers — .susi-layout height should stay stable (0px delta on branch preview)
  4. Short viewport: no inner scrollbar on .susi-tab-panels; page/dialog scrolls instead

Modal variants (both After modal QA URLs):

  1. Open each modal link on susi-light-copy and susi-light-copy-copy
  2. Wrapper should be pre-sized before hydrate — no upward jump
  3. DevTools: document.querySelector('.dialog-modal .susi-light')?.dataset.susiWrapperProfile (e.g. b2b-email-first, legacy-edu-hed)
  4. On a short viewport, confirm the dialog scrolls and the form is not clipped (no inner scrollbar on .susi-wrapper)

Priority modals: b2b-default, b2b-email-first, legacy-edu-express, edu-default, edu-hed, student-default

Before (stage): Visible layout shift as susi-sentry-light grows on hydration

After: Height reserved from first paint; may retain min-height slack after content settles (acceptable empty space vs CLS)


Potential Regressions

Flavors to spot-check: b2b, b2b email-first, b2b email-only, edu (+ _HED / _Business client IDs), student, tabs, simplified, legacy bare buildEdu.


Additional Notes

Files changed (vs stage):

  • susi-light.css:root --susi-wrapper-* / --susi-tabs-* tokens; tab panel min-height; modal .susi-wrapper min-height; legacy bare-block rule
  • susi-light.jsapplyModalWrapperReserve, applyTabsReserveFromAuthoring, profile resolvers, MODAL_WRAPPER_FALLBACK_PX, aria-hidden/inert on tabs
  • test/blocks/susi-light/susi-light.test.js — profile + reserve unit tests (23 tests)
  • nala/blocks/susi-light/ — tab-switch height assertion (≤8px tolerance)

Heights are static constants (not per social-login count / AB bucket). Re-measure on draft QA pages if SUSI layout changes.

@aem-code-sync

aem-code-sync Bot commented Jun 2, 2026

Copy link
Copy Markdown

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.

  • Re-run all PSI checks
  • Re-run failed PSI checks
  • Re-sync branch
Commits

@aem-code-sync

aem-code-sync Bot commented Jun 2, 2026

Copy link
Copy Markdown

@saurabhsircar11 saurabhsircar11 changed the title fix(susi-light): pre-reserve wrapper heights to eliminate CLS on login page and modals MWPW-178300: pre-reserve wrapper heights to eliminate CLS on login page and modals Jun 2, 2026
@aem-code-sync
aem-code-sync Bot temporarily deployed to MWPW-178300-susi-light-height June 2, 2026 15:06 Inactive
@aem-code-sync
aem-code-sync Bot temporarily deployed to MWPW-178300-susi-light-height June 3, 2026 10:38 Inactive
@aem-code-sync
aem-code-sync Bot temporarily deployed to MWPW-178300-susi-light-height June 3, 2026 13:55 Inactive
@saurabhsircar11
saurabhsircar11 marked this pull request as ready for review June 3, 2026 14:00
@saurabhsircar11
saurabhsircar11 marked this pull request as draft June 3, 2026 16:35
@saurabhsircar11
saurabhsircar11 marked this pull request as ready for review June 3, 2026 17:11
@saurabhsircar11 saurabhsircar11 added the Ready for Review Ready for peer review. label Jun 3, 2026
@meganthecoder

Copy link
Copy Markdown
Contributor

@saurabhsircar11 I'm seeing a lot of extra spacing. In screenshot, After vs Before:
Screenshot 2026-06-08 at 8 21 15 AM
Screenshot 2026-06-08 at 8 21 39 AM

@aem-code-sync
aem-code-sync Bot temporarily deployed to MWPW-178300-susi-light-height June 9, 2026 05:27 Inactive
@aem-code-sync
aem-code-sync Bot temporarily deployed to MWPW-178300-susi-light-height June 9, 2026 06:08 Inactive
@saurabhsircar11

saurabhsircar11 commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator Author

@meganthecoder Thanks for pointing it out! I have tightened the min-height values to reduce the extra space — also removed the earlier 8px buffer. Updated the PR description with the latest changes.

The Nala tabs fixture Educators panel can exceed the login-page floor
with min-height-only slots. Step-5 now verifies the reserve is applied
and the tab panel slot stays at or above that floor when switching tabs.

Co-authored-by: Cursor <cursoragent@cursor.com>
@aem-code-sync
aem-code-sync Bot temporarily deployed to MWPW-178300-susi-light-height June 9, 2026 07:11 Inactive
@aem-code-sync
aem-code-sync Bot temporarily deployed to MWPW-178300-susi-light-height June 22, 2026 17:44 Inactive

@meganthecoder meganthecoder left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for all the work. I know that this was Jingle's idea, but I still question the overall premise/approach. This seems difficult to maintain with all the hardcoded values. What about different languages that might be wrap? Also, we know the susi styling is different on stage and prod. Will these match? @echen-adobe what do you think?

Comment on lines +5 to +28
--susi-tabs-panel-height: 521px;
--susi-tabs-panel-standard: 508px;
--susi-tabs-panel-edu-express: 521px;
--susi-tabs-wrapper-standard: 458px;
--susi-tabs-wrapper-edu-express: 367px;
--susi-tabs-panel-buffer: 0px;
--susi-wrapper-standard: 484px;
--susi-wrapper-b2b-default: 484px;
--susi-wrapper-b2b-hed: 291px;
--susi-wrapper-b2b-business: 393px;
--susi-wrapper-b2b-email-first: 409px;
--susi-wrapper-b2b-email-only: 294px;
--susi-wrapper-edu-default: 484px;
--susi-wrapper-edu-hed: 333px;
--susi-wrapper-edu-business: 393px;
--susi-wrapper-edu-express: 400px;
--susi-wrapper-student-default: 462px;
--susi-wrapper-student-hed: 422px;
--susi-wrapper-student-business: 422px;
--susi-wrapper-student-email-only: 294px;
--susi-wrapper-student: 422px;
--susi-wrapper-simplified: 400px;
--susi-wrapper-legacy-edu-express: 545px;
--susi-wrapper-legacy-edu-hed: 477px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like most of these aren't being used and that they're duplicated in the js. It'd be better if there was a single source of truth. Either have the js use/apply these css vars or remove the unused css vars.

@saurabhsircar11 saurabhsircar11 Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks Megan, removed the unused tokens from the CSS and kept the heights in the JS as the single source of truth. Also cleaned up a similar dead duplicate on the JS side.

@@ -1,7 +1,31 @@
/* CLS min-height floors (not exact heights) — re-measure: /drafts/sircar/susi-light-copy */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we should include a draft page path in a comment for maintenance reasons

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks Megan, removed the draft path from the comment

saurabhsircar11 and others added 2 commits June 24, 2026 21:06
…truth

- Remove unused per-profile :root CSS vars (duplicated as JS constants);
  per-profile heights now owned solely by JS (MODAL_WRAPPER_FALLBACK_PX /
  TABS_PANEL_HEIGHTS), applied inline.
- Remove dead JS TABS_WRAPPER_HEIGHTS / resolveTabsWrapperMinHeight;
  tab-wrapper heights owned solely by CSS.
- Drop draft page path from CSS comment.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@aem-code-sync
aem-code-sync Bot temporarily deployed to MWPW-178300-susi-light-height June 24, 2026 16:52 Inactive

@echen-adobe echen-adobe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we get test links for international use cases for all of these? Fixed width might not play well with different languages?

@aem-code-sync
aem-code-sync Bot temporarily deployed to MWPW-178300-susi-light-height July 22, 2026 13:12 Inactive
…rements

Measured teacher-fragment content across 18 locales: 508px (latin) to
519px (kr). The 545px floor added up to 37px of bottom whitespace in the
modal; 520px covers the tallest locale with no overflow.
@saurabhsircar11

Copy link
Copy Markdown
Collaborator Author

Hi @echen-adobe,

I went through all the locales where susi-light shows up — here's what I found.

Login pages: checked /{locale}/express/login in all 18 locales. Longer languages do wrap (French takes an extra line, CJK renders a bit taller), but since we only set min-height floors, the card just grows to fit — nothing clips. The fixed widths aren't from this PR, those were already there. Some good ones to try — the card holds its size while SUSI loads instead of jumping like it does on main:

One thing you'll still notice: in es/it/fi (and a bit in fr/de/nl/no/dk/br), switching to the Educators tab causes a jump, because that panel comes back taller than the 521px we reserve. Initial load is fine, it's only the tab switch. We could raise the min-height for those locales but then everyone else gets white space under the card. Main has the same jump today (ours is smaller or equal in every locale, and us/uk/se/id_id/in are now completely stable where main still shifts), so I'd rather keep the balanced floor and maybe do a follow-up that bumps the slot height after the first hydration.

Modals: opened the sign-in modals on /express/business, /express/spotlight/business, /express/learn/students (+ uk) and /in/express/spotlight/education and compared screenshots against main — identical, and the modal now opens at its final size instead of growing after open.

Heads up on something I hit while testing: on the localized business pages (de, fr, es, it, nl, dk, fi, se, no, kr, tw, cn, id_id) the sign-in modal doesn't open at all. The CTA points at the localized susi-light-b2b-email-first fragment which isn't published (404), so the modal has nothing to load. Same behavior on main, so it's not from this PR — but it means I couldn't test the localized b2b modal end-to-end, and it probably needs a content ticket.

To still cover localized modal content, I loaded the published localized fragments directly (susi-light-teacher ×17 locales, susi-edu ×18 — same code path the modal renders) and compared against main: no differences.

Testing also caught one real issue in our own numbers: the legacy-edu-express reserve (545px) was taller than actual content in every locale (508–519px), leaving an empty band at the bottom of the teacher modal. Dropped it to 520px in b819e4c and re-checked all locales.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for Review Ready for peer review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants