Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core: Testing Module UI #29236

Merged
merged 43 commits into from
Oct 1, 2024
Merged

Core: Testing Module UI #29236

merged 43 commits into from
Oct 1, 2024

Conversation

ghengeveld
Copy link
Member

@ghengeveld ghengeveld commented Sep 27, 2024

Closes #29096

What I did

This implements the UI for the Testing Module. It sits docked in the sidebar, floating at the bottom. Notifications were updated to sit in the sidebar as well, just above the Testing Module. The sidebar has a div#sidebar-bottom element with some CSS variables attached to allow integrators such as Chromatic to add custom elements here, whilst retaining a consistent UI.

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!

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 77.7 MB 77.7 MB 0 B - 0%
initSize 152 MB 152 MB 19.1 kB Infinity 0%
diffSize 74.4 MB 74.5 MB 19.1 kB Infinity 0%
buildSize 7.27 MB 7.28 MB 12.3 kB Infinity 0.2%
buildSbAddonsSize 1.87 MB 1.87 MB 651 B Infinity 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.92 MB 1.93 MB 9.47 kB Infinity 0.5%
buildSbPreviewSize 312 kB 312 kB 0 B - 0%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 4.3 MB 4.31 MB 10.1 kB Infinity 0.2%
buildPreviewSize 2.97 MB 2.98 MB 2.14 kB Infinity 0.1%
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 5.5s 21s 15.4s Infinity 🔺73.5%
generateTime 19.6s 19.3s -303ms -Infinity -1.6%
initTime 14.4s 14s -418ms -Infinity -3%
buildTime 13s 9.5s -3s -537ms -Infinity 🔰-37.1%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 8.4s 6.3s -2s -128ms -Infinity 🔰-33.7%
devManagerResponsive 5.4s 4s -1s -324ms -Infinity 🔰-32.4%
devManagerHeaderVisible 719ms 591ms -128ms -Infinity 🔰-21.7%
devManagerIndexVisible 758ms 638ms -120ms -Infinity 🔰-18.8%
devStoryVisibleUncached 1.6s 713ms -897ms -Infinity 🔰-125.8%
devStoryVisible 756ms 639ms -117ms -Infinity 🔰-18.3%
devAutodocsVisible 626ms 570ms -56ms -Infinity 🔰-9.8%
devMDXVisible 655ms 596ms -59ms -Infinity 🔰-9.9%
buildManagerHeaderVisible 574ms 528ms -46ms -Infinity 🔰-8.7%
buildManagerIndexVisible 658ms 594ms -64ms -Infinity 🔰-10.8%
buildStoryVisible 659ms 596ms -63ms -Infinity 🔰-10.6%
buildAutodocsVisible 645ms 483ms -162ms -Infinity 🔰-33.5%
buildMDXVisible 550ms 491ms -59ms -Infinity 🔰-12%

Greptile Summary

This PR implements the UI for the Testing Module in Storybook, focusing on creating a floating container at the bottom of the sidebar with test provider information and controls.

  • Added new TestingModule component in code/core/src/manager/components/sidebar/TestingModule.tsx
  • Updated SidebarBottom component to include TestingModule and notifications
  • Implemented collapsible UI with run/stop buttons and error/warning filters
  • Added 'runnable' and 'watchable' properties to test provider configuration in code/addons/test/src/manager.tsx
  • Updated NotificationList and NotificationItem components for improved responsiveness and styling

@ghengeveld ghengeveld changed the base branch from next to unified-ui-testing September 27, 2024 15:33
Copy link
Contributor

Fails
🚫

PR is not labeled with one of: ["cleanup","BREAKING CHANGE","feature request","bug","documentation","maintenance","build","dependencies"]

🚫

PR is not labeled with one of: ["ci:normal","ci:merged","ci:daily","ci:docs"]

🚫 PR title must be in the format of "Area: Summary", With both Area and Summary starting with a capital letter Good examples: - "Docs: Describe Canvas Doc Block" - "Svelte: Support Svelte v4" Bad examples: - "add new api docs" - "fix: Svelte 4 support" - "Vue: improve docs"

Generated by 🚫 dangerJS against a43f27e

Copy link

nx-cloud bot commented Sep 27, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 0c11df3. 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

Sent with 💌 from NxCloud.

@ghengeveld ghengeveld marked this pull request as ready for review October 1, 2024 06:43
@ghengeveld ghengeveld changed the title Testing Module UI Core: Testing Module UI Oct 1, 2024
@yannbf yannbf added the core label Oct 1, 2024
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

29 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings

Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

Awesome stuff!!

Here's some testing feedback we should fix before merging (if applicable, else we should create separate tickets for them):

Watch mode state is not shown upon click

As you can see, the button won't change into an active/inactive state. Although watch mode toggling works, it doesn't feel like it in the UI

Expected behavior: The eye icon should be switched to active/inactive state upon state change.
2024-10-01 09 07 07

Loading state is not shown on test run

Although there is a story for the loading state with a spinner and where the icon shows a square, clicking on the run button doesn't trigger that
Expected behavior: The spinner should shown upon test runs, as well as the stop icon.
2024-10-01 09 14 13

Run all tests does not run tests

It does spawn vitest, but no tests are run
Expected behavior: All tests should be triggered, as well as the loading state of the components should be toggled.
2024-10-01 09 16 19

Error/warning filter colors are not applied

Expected behavior: Colors should be applied correctly

image

That's because there are other colors overwriting the ones defined in the component, this happens as you set the Button to have a ghost variant in this line of code:

The testing module is available on Mobile

Although it looks relatively good (though the shadow seems excessive on dark mode), I thought the testing module should not appear at all on mobile?

Expected behavior: Testing module should not be rendered on mobile (unless decided otherwise)

It also clashes with the notification element:

Notifications look off on mobile (dark mode)

Because the notifications appear on any view on mobile, when in dark mode they have a weird shadow that only makes sense if they're rendered on the sidebar:

@ghengeveld
Copy link
Member Author

Watch mode state is not shown upon click
We'll handle this when we deal with status updates in a later PR.

Loading state is not shown on test run
Same.

Run all tests does not run tests
Fixed.

Error/warning filter colors are not applied
Fixed.

The testing module is available on Mobile
Fixed.

Notifications look off on mobile (dark mode)
Fixed.

@yannbf yannbf added ci:merged Run the CI jobs that normally run when merged. and removed ci:normal labels Oct 1, 2024
@ghengeveld ghengeveld merged commit d24cab3 into unified-ui-testing Oct 1, 2024
66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:merged Run the CI jobs that normally run when merged. core feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants