feat: Widgetize app layout skeleton event base #3497

georgylobko · 2025-05-13T08:22:40Z

Description

Related links, issue #, if available: n/a

How has this been tested?

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

Changes include appropriate documentation updates.
Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
Changes do not include unsupported browser features, see CONTRIBUTING.md.
Changes were manually tested for accessibility, see accessibility guidelines.

Security

If the code handles URLs: all URLs are validated through the checkSafeUrl function.

Testing

Changes are covered with new/existing unit tests?
Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

codecov · 2025-05-13T10:08:55Z

Codecov Report

❌ Patch coverage is 95.07042% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.90%. Comparing base (8cda3da) to head (fef642b).
⚠️ Report is 18 commits behind head on main.

Files with missing lines	Patch %	Lines
src/app-layout/visual-refresh-toolbar/internal.tsx	50.00%	8 Missing and 1 partial ⚠️
src/breadcrumb-group/internal.tsx	75.00%	3 Missing and 1 partial ⚠️
src/app-layout/utils/use-focus-control.ts	92.50%	3 Missing ⚠️
...p-layout/visual-refresh-toolbar/use-app-layout.tsx	97.69%	3 Missing ⚠️
src/app-layout/utils/defer.ts	90.00%	1 Missing ⚠️
...p-layout/visual-refresh-toolbar/skeleton/index.tsx	98.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3497      +/-   ##
==========================================
- Coverage   96.96%   96.90%   -0.07%     
==========================================
  Files         820      828       +8     
  Lines       23786    23999     +213     
  Branches     8337     8432      +95     
==========================================
+ Hits        23064    23256     +192     
+ Misses        716      692      -24     
- Partials        6       51      +45

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

just-boris · 2025-07-11T16:44:45Z

pages/utils/iframe-wrapper.tsx

+      ReactDOM.render(<AppComponent />, innerAppRoot);
+      return () => {
+        syncClassesCleanup();
+        ReactDOM.unmountComponentAtNode(innerAppRoot);


This cleanup does not work because it is inside a timeout now

just-boris · 2025-07-11T16:45:49Z

src/app-layout-toolbar/__tests__/analytics-metadata.test.tsx

@@ -154,6 +157,9 @@ describe('AppLayoutToolbar renders correct analytics metadata', () => {
          toolsClose: 'close tools',
        },
      });
+      await waitFor(() => {
+        expect(wrapper.findToolsToggle()).toBeTruthy();
+      });
      const toolsTrigger = wrapper.findToolsToggle().getElement();
      validateComponentNameAndLabels(toolsTrigger, {});
      expect(getGeneratedAnalyticsMetadata(toolsTrigger)).toEqual({


Does it have any issues for analytics if these metadata is rendered too late

just-boris · 2025-07-11T16:49:38Z

src/app-layout/__tests__/common.test.tsx

    const { wrapper } = renderComponent(<AppLayout />);

    expect(wrapper.findNavigationToggle()).toBeTruthy();
    expect(wrapper.findNavigation()).toBeTruthy();
    expect(wrapper.findNavigationClose()).toBeTruthy();
-    expect(wrapper.findToolsToggle()).toBeTruthy();
+    await waitFor(() => {


If everything becomes async, this is a breaking change for our consumers.

All of their unit tests will fail, won't they?

Explored deeper, there seems to be a solution. One simple change allowed to remove some of them: https://github.com/cloudscape-design/components/pull/3648/files?w=1#diff-3cfa742d60cd325bf84e2694ddf9a04bf3ebdda6cd27919a6dec23caac3953d0

just-boris · 2025-07-11T16:53:18Z

src/app-layout/__tests__/skeleton.test.tsx

@@ -26,8 +26,8 @@ jest.mock('../../../lib/components/internal/widgets', () => ({
  createWidgetizedComponent: createWidgetizedComponentMock,
 }));

-describeEachAppLayout({ themes: ['refresh-toolbar'] }, () => {
-  it('renders complete component by default', () => {
+describeEachAppLayout({ themes: ['refresh-toolbar'], skipInitialTest: true }, () => {


What happens with initial test in this case?

Instead of skipping the test, can we rewrite this test so it only asserts synchronously available parts?

just-boris · 2025-07-11T16:54:06Z

src/app-layout/__tests__/skeleton.test.tsx

@@ -65,7 +66,6 @@ describeEachAppLayout({ themes: ['refresh-toolbar'] }, () => {
      expect(wrapper.findToolbar()).toBeFalsy();
      expect(wrapper.findNavigation()).toBeFalsy();
      expect(wrapper.findBreadcrumbs()).toBeFalsy();
-      expect(wrapper.find(getFunnelKeySelector('funnel-name'))).toBeTruthy();


Why is this removed? This is one of critical feature

just-boris · 2025-07-11T17:13:32Z

src/app-layout/utils/use-focus-control.ts

@@ -83,7 +91,10 @@ export function useMultipleFocusControl(
  const shouldFocus = useRef(false);

  useEffect(() => {
-    doFocus(activeDrawersIds[0]);
+    const drawerId = activeDrawersIds[0];
+    refs.current[drawerId]?.focusPromise?.promise?.then(() => {


If we introduced a separate useAsyncFocusControl, do we really need to change this one?

just-boris · 2025-07-11T17:14:50Z

src/app-layout/utils/use-focus-control.ts

+  };
+
+  useEffect(() => {
+    focusPromise.current.promise.then(() => {


Should we cancel this waiting if the state changes before the promise is resolved?

just-boris · 2025-07-11T17:17:43Z

src/app-layout/visual-refresh-toolbar/internal.tsx

+    useEffect(() => {
+      const timeoutId = setTimeout(() => {
+        setMount(true);
+      }, 1000);
+
+      return () => clearTimeout(timeoutId);
+    }, []);


This code runs is production, even if enableDelayedComponents = false

To avoid this, the check should happen right under createAppLayoutPart

export function createAppLayoutPart({ Component }) { if(!enableDelayedComponents) { return Component; } // delayed mock }

just-boris · 2025-07-11T17:18:49Z

src/app-layout/visual-refresh-toolbar/skeleton/widget-slots/use-skeleton-slots-attributes.tsx

+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0


Double copyright header

just-boris · 2025-07-11T17:19:26Z

src/breadcrumb-group/internal.tsx

+const enableDelayedComponents = false;
+
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export function createAppLayoutPart({ Component }: { Component: React.JSXElementConstructor<any> }) {


Should we reuse it from the other place?

just-boris · 2025-07-14T10:18:11Z

src/app-layout/visual-refresh-toolbar/skeleton/widget-slots/side-page-slot.tsx

+          // type casting is safe here since 'drawers' only exists at runtime after appLayoutInternals
+          // has been initialized
+          <AppLayoutDrawer appLayoutInternals={appLayoutInternals as AppLayoutInternals} />


Sometimes you do appLayoutInternals as AppLayoutInternals and sometimes appLayoutInternals!, why it is different?

Can we have a unified way to check "is app layout widget loaded", so all code down the line knows that accessing values is safe

just-boris · 2025-07-14T10:21:21Z

src/app-layout/visual-refresh-toolbar/skeleton/widget-slots/side-page-slot.tsx

+          sharedStyles['with-motion-horizontal'],
+          resolvedNavigationOpen && !toolsOpen && styles['unfocusable-mobile'],
+          toolsOpen && styles['tools-open'],
+          drawerExpandedMode && styles.hidden


Wasn't the original plan to abstract away all features and use only generic names in skeleton code?

So this is not the drawerExpandedMode but isPanelHidden, which joins all !toolsOpen, drawerExpandedMode and everything else that may require hiding the panel?

just-boris · 2025-07-14T10:24:38Z

src/app-layout/visual-refresh-toolbar/skeleton/widget-slots/side-page-slot.tsx

+      </div>
+      <div className={clsx(styles['global-tools'], !globalToolsOpen && styles['panel-hidden'])}>


Didn't we want to move this panel separation logic into the widget to make it more generic here?

just-boris · 2025-07-14T10:25:48Z

src/app-layout/visual-refresh-toolbar/skeleton/widget-slots/bottom-page-content-slot.tsx

+  const { splitPanelPosition, appLayoutInternals, splitPanelInternals } = appLayoutState ?? {};
+  return (
+    <>
+      {splitPanelPosition === 'bottom' && (


How can we insert a new bottom panel here without touching the skeleton if it has a hard-coded mention of the split panel?

just-boris · 2025-07-14T13:50:12Z

src/app-layout/__tests__/common.test.tsx

    const { wrapper } = renderComponent(<AppLayout />);

    expect(wrapper.findNavigationToggle()).toBeTruthy();
    expect(wrapper.findNavigation()).toBeTruthy();
    expect(wrapper.findNavigationClose()).toBeTruthy();
-    expect(wrapper.findToolsToggle()).toBeTruthy();
+    await waitFor(() => {


Explored deeper, there seems to be a solution. One simple change allowed to remove some of them: https://github.com/cloudscape-design/components/pull/3648/files?w=1#diff-3cfa742d60cd325bf84e2694ddf9a04bf3ebdda6cd27919a6dec23caac3953d0

chore: Move skeleton elements attributes to a widgetized hook chore: Move skeleton slots to widgets chore: Widgetize all remaining components feat: AppLayout state as widget chore: small fix chore: small fix chore: Small refactoring chore: Pull react-reverse-portal as src util chore: Revert unrelated changes in runtime-drawers.page.tsx fix: SSR tests fix: Skeleton part tests feat: Deliver skeleton slots attributes as a widget part fix: Unit tests chore: Update snapshots chore: Tests for reverse portal chore: Additional tests for reverse portal fix: Integ tests chore: Skip hash for class names in widget contract tests fix: Tests chore: Wrap up chore: Fallbacks for app layout component when is in loading state (avoiding layout cumulative shifting) chore: Delay breadcrumbs rendering until app layout state is loaded feat: Event-base prop passing Fix u tests and remove unused reverse-portal feat: Split app layout state into widgetized and built-in parts chore: Merge expanded mode chore: Merge from main fix: useMergeRefs import fix: remove unnecessary import fix: Navigation fallback for useMultiAppLayout chore: Refactoring, fixed tools test chore: Refactoring, fixed skeleton test chore: Update snapshots fix: header-variant test fix: toolbar test fix: multi-layout.test.tsx fix: slots.test.tsx fix: desktop.test.tsx chore: Update snapshots fix: split-panel.test.tsx fix: main.test.tsx fix: drawers.test.tsx fix: common.test.tsx fix: analytics-metadata.test.tsx chore: Update snapshots chore: Update snapshots feat: Async focus control chore: Migrate useUniqueId import to @cloudscape-design/component-toolkit fix: onMount type issue feat: Introduce useAsyncFocusControl chore: Clean up style hash in widget-contract-split-panel.test.tsx chore: Cleanup multi-layout-with-hidden-instances-iframe.page.tsx chore: Small test adjustment chore: Suppress an overlay for runtime errors in webpack config fix: Eslint error fix: AppLayoutPartLoader mounting issue chore: Increased a timeout for multi-layout.test.tsx chore: Types for a state manager chore: Wrapping up (type adjustments, snapshot upd) chore: Turn focusSplitPanel into async function (wait for an element before focusing on it) feat: Custom feature flag appLayoutDelayedWidget to test async widget behavior chore: Integ test for checking ensure the layout is not shifting after widget loading chore: Upd snapshot tests fix: Duplicate 'client' key in webpack.config.base.cjs fix: analytics metadata

just-boris · 2025-08-08T14:59:15Z

The work moved to #3721

georgylobko deployed to dev-pages May 13, 2025 10:08 — with GitHub Actions View deployment

georgylobko deployed to dev-pages May 13, 2025 10:18 — with GitHub Actions View deployment

georgylobko deployed to dev-pages June 3, 2025 12:13 — with GitHub Actions View deployment

georgylobko deployed to dev-pages June 4, 2025 08:31 — with GitHub Actions View deployment

georgylobko deployed to dev-pages July 4, 2025 12:57 — with GitHub Actions View deployment

georgylobko deployed to dev-pages July 4, 2025 14:35 — with GitHub Actions View deployment

georgylobko deployed to dev-pages July 7, 2025 08:00 — with GitHub Actions View deployment

georgylobko deployed to dev-pages July 7, 2025 13:07 — with GitHub Actions View deployment

georgylobko deployed to dev-pages July 7, 2025 16:08 — with GitHub Actions View deployment

georgylobko deployed to dev-pages July 8, 2025 11:33 — with GitHub Actions View deployment

georgylobko deployed to dev-pages July 8, 2025 11:54 — with GitHub Actions View deployment

georgylobko deployed to dev-pages July 9, 2025 13:09 — with GitHub Actions View deployment

georgylobko deployed to dev-pages July 10, 2025 08:42 — with GitHub Actions View deployment

georgylobko deployed to dev-pages July 10, 2025 10:19 — with GitHub Actions View deployment

georgylobko deployed to dev-pages July 10, 2025 10:44 — with GitHub Actions View deployment

georgylobko deployed to dev-pages July 11, 2025 11:51 — with GitHub Actions View deployment

georgylobko marked this pull request as ready for review July 11, 2025 12:22

georgylobko requested a review from a team as a code owner July 11, 2025 12:22

georgylobko requested review from cansuaa and just-boris and removed request for a team July 11, 2025 12:22

just-boris reviewed Jul 11, 2025

View reviewed changes

just-boris reviewed Jul 14, 2025

View reviewed changes

georgylobko force-pushed the feat/widgetize-app-layout-skeleton-event-base branch from f344749 to 9c84089 Compare July 22, 2025 15:18

georgylobko deployed to dev-pages July 22, 2025 15:27 — with GitHub Actions View deployment

georgylobko force-pushed the feat/widgetize-app-layout-skeleton-event-base branch from 9c84089 to 98d26f7 Compare July 22, 2025 15:28

georgylobko deployed to dev-pages July 22, 2025 15:37 — with GitHub Actions View deployment

just-boris force-pushed the feat/widgetize-app-layout-skeleton-event-base branch from 7508689 to 51d4995 Compare July 23, 2025 12:07

just-boris deployed to dev-pages July 23, 2025 12:48 — with GitHub Actions View deployment

just-boris force-pushed the feat/widgetize-app-layout-skeleton-event-base branch 2 times, most recently from 378f8a6 to 171d166 Compare July 24, 2025 13:55

just-boris deployed to dev-pages July 24, 2025 14:04 — with GitHub Actions View deployment

just-boris force-pushed the feat/widgetize-app-layout-skeleton-event-base branch 2 times, most recently from 4cbcf55 to ad8f1d7 Compare July 25, 2025 15:18

just-boris deployed to dev-pages July 25, 2025 16:02 — with GitHub Actions View deployment

just-boris force-pushed the feat/widgetize-app-layout-skeleton-event-base branch from 497adbb to 7f5aec4 Compare July 29, 2025 10:47

just-boris deployed to dev-pages July 29, 2025 10:56 — with GitHub Actions View deployment

just-boris force-pushed the feat/widgetize-app-layout-skeleton-event-base branch from 7f5aec4 to 54668d4 Compare July 29, 2025 17:33

just-boris deployed to dev-pages July 29, 2025 18:18 — with GitHub Actions View deployment

just-boris force-pushed the feat/widgetize-app-layout-skeleton-event-base branch from 54668d4 to 8520636 Compare July 30, 2025 10:28

just-boris deployed to dev-pages July 30, 2025 11:57 — with GitHub Actions View deployment

just-boris force-pushed the feat/widgetize-app-layout-skeleton-event-base branch from 6ab60c6 to e9a4d1b Compare July 30, 2025 15:03

just-boris deployed to dev-pages July 30, 2025 15:12 — with GitHub Actions View deployment

georgylobko and others added 6 commits August 1, 2025 18:59

export new widgetized parts

468a1cd

increase size limit

1c88ade

rename widget exports

77f0e4b

make everything sync by default

983824d

temporarily skip some tests

fef642b

just-boris force-pushed the feat/widgetize-app-layout-skeleton-event-base branch from e9a4d1b to fef642b Compare August 1, 2025 16:59

just-boris deployed to dev-pages August 1, 2025 17:08 — with GitHub Actions View deployment

just-boris closed this Aug 8, 2025

		// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
		// SPDX-License-Identifier: Apache-2.0

		</div>
		<div className={clsx(styles['global-tools'], !globalToolsOpen && styles['panel-hidden'])}>

feat: Widgetize app layout skeleton event base #3497

feat: Widgetize app layout skeleton event base #3497

Uh oh!

Conversation

georgylobko commented May 13, 2025

Description

How has this been tested?

Correctness

Security

Testing

Uh oh!

codecov bot commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

just-boris commented Aug 8, 2025

Uh oh!

Uh oh!

codecov bot commented May 13, 2025 •

edited

Loading