-
Notifications
You must be signed in to change notification settings - Fork 3.3k
misc: Assertion dropdown UI update #31598
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the assertion dropdown UI by refactoring the types, improving the menu mounting logic, and enhancing accessibility and styling in the studio components.
- Refactored assertion types to use an interface for better clarity.
- Improved event handling and throttling logic in the mounter.
- Enhanced accessibility features and styling in the AssertionsMenu and related components.
Reviewed Changes
Copilot reviewed 7 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
packages/app/src/runner/studio/types.ts | Refactored assertion types and updated function type signatures. |
packages/app/src/runner/studio/mounter.ts | Introduced event throttling and helper functions for mounting. |
packages/app/src/runner/studio/AssertionsMenu.cy.tsx | Added tests to validate UI behavior and assertions functionality. |
packages/app/src/runner/studio/AssertionsMenu.ce.vue | Updated header, styling, and accessibility attributes. |
packages/app/src/runner/studio/AssertionType.ce.vue | Enhanced accessibility through ARIA attributes and keyboard events. |
packages/app/src/runner/studio/AssertionOptions.ce.vue | Improved option handling with key generation and truncation logic. |
Files not reviewed (4)
- packages/app/package.json: Language not supported
- packages/app/src/runner/studio/assertions-style.scss: Language not supported
- packages/frontend-shared/package.json: Language not supported
- packages/reporter/src/lib/variables.scss: Language not supported
Comments suppressed due to low confidence (1)
packages/app/src/runner/studio/mounter.ts:44
- The throttling logic subtracts the event time from the last timestamp, which can result in a negative value. Consider using 'e.timeStamp - lastTimeStamp' to accurately throttle repeated events.
function shouldThrottleEvent (e: MouseEvent): boolean { return lastTarget === e.target && lastTimeStamp - e.timeStamp < EVENT_THROTTLE_MS }
cypress
|
Project |
cypress
|
Branch Review |
assertion-dropdown-ui-update
|
Run status |
|
Run duration | 18m 54s |
Commit |
|
Committer | Jennifer Shehane |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
12
|
|
1232
|
|
0
|
|
32156
|
View all changes introduced in this branch ↗︎ |
UI Coverage
46.24%
|
|
---|---|
|
188
|
|
166
|
Accessibility
92.73%
|
|
---|---|
|
3 critical
9 serious
2 moderate
2 minor
|
|
888
|
@@ -0,0 +1,58 @@ | |||
import { getOrCreateHelperDom } from './dom' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function seemed to be completely untested, so I added a test for this. Many of these runner 'helper' files are untested actually so not a lot of conventions to pull from.
@@ -5,14 +5,7 @@ import AssertionOptions from './AssertionOptions.ce.vue' | |||
import { getOrCreateHelperDom, getSelectorHighlightStyles } from '../dom' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a straight refactor + adding types. None of the logic should be changed here.
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
This PR updates the assertions dropdown to match our 'dark mode' styles in the rest of the App.
Additionally this PR:
Steps to test
yarn cypress:open
experimentalStudio: true
set in thee2e
config of thecypress.config.ts
file.How has the user experience changed?
Before
After
PR Tasks
cypress-documentation
? - I do need to open a PR to change this page - https://docs.cypress.io/app/guides/cypress-studio but don't want to block this PR.