Addon Test: Support viewport globals #29319

yannbf · 2024-10-10T09:51:02Z

What I did

This PR adds support for the new globals API for viewports, so that users that use the new test addon can get Playwright to use the correct dimensions respecting the viewports addon, both via parameters (legacy) API as well as globals (new) API

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

stories
unit tests
integration tests
end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

There are tests for all use cases, but to test it manually you can use a story like the following (using both parameters and globals apis):

const customViewports = {
  sized: {
    name: 'Sized',
    styles: {
      width: '380px',
      height: '500px',
    },
  },
};
const parametersApi = {
  defaultViewport: 'sized',
  viewports: customViewports,
}
const globalsApi = {
  options: customViewports,
}

export const ExpectedSuccess = {
  parameters: {
    viewport: {
      ...parametersApi,
      ...globalsApi
    },
  },
  globals: { viewport: { value: 'sized' } },
};

Then run Vitest browser mode tests visually, and notice the viewport changing in size.

Documentation

Add or update documentation reflecting your changes
If you are deprecating/removing a feature, make sure to update
MIGRATION.MD

Checklist for Maintainers

When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts
Make sure this PR contains one of the labels below:
Available labels
- bug: Internal changes that fixes incorrect behavior.
- maintenance: User-facing maintenance tasks.
- dependencies: Upgrading (sometimes downgrading) dependencies.
- build: Internal-facing build tooling & test updates. Will not show up in release changelog.
- cleanup: Minor cleanup style change. Will not show up in release changelog.
- documentation: Documentation only changes. Will not show up in release changelog.
- feature request: Introducing a new feature.
- BREAKING CHANGE: Changes that break compatibility in some way with current major version.
- other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name	before	after	diff	z	%
createSize	0 B	0 B	0 B	-	-
generateSize	78.7 MB	78.7 MB	0 B	0.81	0%
initSize	147 MB	147 MB	-351 B	-1.39	0%
diffSize	68.3 MB	68.3 MB	-351 B	-1.38	0%
buildSize	7.1 MB	7.09 MB	-2.47 kB	-0.79	0%
buildSbAddonsSize	1.79 MB	1.78 MB	-2.13 kB	-0.89	-0.1%
buildSbCommonSize	195 kB	195 kB	0 B	-	0%
buildSbManagerSize	1.85 MB	1.85 MB	-345 B	-0.87	0%
buildSbPreviewSize	271 kB	271 kB	-2 B	-0.9	0%
buildStaticSize	0 B	0 B	0 B	-	-
buildPrebuildSize	4.1 MB	4.1 MB	-2.47 kB	-0.88	-0.1%
buildPreviewSize	3 MB	3 MB	0 B	1.41	0%
testBuildSize	0 B	0 B	0 B	-	-
testBuildSbAddonsSize	0 B	0 B	0 B	-	-
testBuildSbCommonSize	0 B	0 B	0 B	-	-
testBuildSbManagerSize	0 B	0 B	0 B	-	-
testBuildSbPreviewSize	0 B	0 B	0 B	-	-
testBuildStaticSize	0 B	0 B	0 B	-	-
testBuildPrebuildSize	0 B	0 B	0 B	-	-
testBuildPreviewSize	0 B	0 B	0 B	-	-

name	before	after	diff	z	%
createTime	12.5s	13.7s	1.2s	-0.31	8.8%
generateTime	25.8s	20.5s	-5s -326ms	-0.41	-26%
initTime	19s	12.9s	-6s -46ms	-1.59	🔰-46.6%
buildTime	9.1s	8.3s	-805ms	-1.34	🔰-9.6%
testBuildTime	0ms	0ms	0ms	-	-
devPreviewResponsive	6.6s	5.9s	-736ms	-0.71	-12.4%
devManagerResponsive	4.2s	3.8s	-418ms	-0.78	-10.8%
devManagerHeaderVisible	651ms	572ms	-79ms	-0.55	-13.8%
devManagerIndexVisible	684ms	608ms	-76ms	-0.52	-12.5%
devStoryVisibleUncached	941ms	918ms	-23ms	-0.74	-2.5%
devStoryVisible	682ms	610ms	-72ms	-0.5	-11.8%
devAutodocsVisible	556ms	531ms	-25ms	-0.43	-4.7%
devMDXVisible	572ms	570ms	-2ms	0.58	-0.4%
buildManagerHeaderVisible	629ms	493ms	-136ms	-1.54	🔰-27.6%
buildManagerIndexVisible	667ms	569ms	-98ms	-0.24	-17.2%
buildStoryVisible	668ms	570ms	-98ms	-0.72	-17.2%
buildAutodocsVisible	540ms	531ms	-9ms	0.01	-1.7%
buildMDXVisible	529ms	513ms	-16ms	-0.22	-3.1%

Greptile Summary

This PR adds support for viewport globals in the Storybook addon testing framework, enhancing the ability to test responsive designs across different viewport sizes.

Updated setViewport function in viewports.ts to handle both parameters and globals for viewport settings
Added new tests in viewports.test.ts to cover the globals API and various viewport dimension formats
Modified testStory function in test-utils.ts to pass both parameters and globals to setViewport
Updated ComposedStoryFn type in composedStory.ts to include a globals property
Removed '!vitest' tags from some stories in tabs.stories.tsx and Sidebar.stories.tsx to enable testing with viewport globals

nx-cloud · 2024-10-10T09:55:24Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit bba2178. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 1 target

nx run-many -t build --parallel=3

Sent with 💌 from NxCloud.

greptile-apps

_{9 file(s) reviewed, 6 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2024-10-10T17:19:01Z

code/addons/test/src/vitest-plugin/viewports.test.ts

+      const viewportsParam: any = {
+        defaultViewport: 'nonExistentViewport',
+      };


style: using 'any' type here might be too permissive

greptile-apps · 2024-10-10T17:19:02Z

code/addons/test/src/vitest-plugin/viewports.test.ts

+      const viewportsParam: any = {
+        viewports: INITIAL_VIEWPORTS,
+        // supported by default in addon viewports


style: using 'any' type here might be too permissive

greptile-apps · 2024-10-10T17:20:51Z

code/addons/test/src/vitest-plugin/viewports.ts

 export interface ViewportsParam {
-  defaultViewport: string;
-  viewports: ViewportMap;
+  defaultViewport?: string;
+  viewports?: ViewportMap;
+  options?: ViewportMap;
+  disable?: boolean;
+  disabled?: boolean;
+}


style: Consider adding JSDoc comments to explain the purpose of each property in the ViewportsParam interface

greptile-apps · 2024-10-10T17:20:51Z

code/addons/test/src/vitest-plugin/viewports.ts

+export interface ViewportsGlobal {
+  value?: string;
+  disable?: boolean;
 }


style: Add JSDoc comments to explain the purpose of the ViewportsGlobal interface and its properties

greptile-apps · 2024-10-10T17:20:52Z

code/addons/test/src/vitest-plugin/viewports.ts

@@ -47,8 +57,18 @@ const parseDimension = (value: string, dimension: 'width' | 'height') => {
  }
 };

-export const setViewport = async (viewportsParam: ViewportsParam = {} as ViewportsParam) => {
-  const defaultViewport = viewportsParam.defaultViewport;
+export const setViewport = async (parameters: Parameters = {}, globals: Globals = {}) => {


style: Update function signature to use more specific types instead of Parameters and Globals

greptile-apps · 2024-10-10T17:20:52Z

code/addons/test/src/vitest-plugin/viewports.ts

  const viewports = {
    ...MINIMAL_VIEWPORTS,
    ...viewportsParam.viewports,
+    ...viewportsParam.options,
  };


logic: Merging viewports might overwrite properties. Ensure this is the intended behavior

yannbf added feature request ci:normal addon: test labels Oct 10, 2024

yannbf self-assigned this Oct 10, 2024

support globals and viewports global config in portable stories

bba2178

yannbf force-pushed the yann/support-viewport-globals-in-ps branch from 733fa26 to bba2178 Compare October 10, 2024 17:16

yannbf marked this pull request as ready for review October 10, 2024 17:16

greptile-apps bot reviewed Oct 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addon Test: Support viewport globals #29319

Addon Test: Support viewport globals #29319

yannbf commented Oct 10, 2024 •

edited by storybook-pr-benchmarking bot

Loading

nx-cloud bot commented Oct 10, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Oct 10, 2024

greptile-apps bot Oct 10, 2024

greptile-apps bot Oct 10, 2024

greptile-apps bot Oct 10, 2024

greptile-apps bot Oct 10, 2024

greptile-apps bot Oct 10, 2024

Addon Test: Support viewport globals #29319

Are you sure you want to change the base?

Addon Test: Support viewport globals #29319

Conversation

yannbf commented Oct 10, 2024 • edited by storybook-pr-benchmarking bot Loading

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

Greptile Summary

nx-cloud bot commented Oct 10, 2024 • edited Loading

☁️ Nx Cloud Report

greptile-apps bot left a comment

Choose a reason for hiding this comment

greptile-apps bot Oct 10, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 10, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 10, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 10, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 10, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 10, 2024

Choose a reason for hiding this comment

yannbf commented Oct 10, 2024 •

edited by storybook-pr-benchmarking bot

Loading

nx-cloud bot commented Oct 10, 2024 •

edited

Loading