-
Notifications
You must be signed in to change notification settings - Fork 3.4k
feat: Runner spec list #18821
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
feat: Runner spec list #18821
Conversation
Thanks for taking the time to open a PR!
|
Test summaryRun details
View run in Cypress Dashboard ➡️ Failures
Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
The only remark I have is we are missing the hover smoothing. |
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.
Cool, looks good, just a few minor comments
packages/graphql/package.json
Outdated
"@packages/types": "0.0.0-development", | ||
"@types/dedent": "^0.7.0", | ||
"chai": "^4.2.0", | ||
"cypress-real-events": "1.4.0", |
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.
Should this be in app/package.json
? I don't think we need this in graphql
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.
Yes it was incorrectly added here.
const target = event.target | ||
if (!target.nextSibling.focus) { |
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.
Just a personal preference/suggestion but should we have the positive case first? eg
if (target.nextSibling.focus) {
// ...
} else {
// ...
}
That way is reads "if" as opposed to "if not"
@elevatebart added the hover smoothing! |
} | ||
</script> | ||
|
||
<style> |
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.
<style> | |
<style scoped lang="scss"> |
<style> | ||
.group:hover path { | ||
transition: all ease-in-out 0.3s; | ||
} | ||
</style> |
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.
Can't merge until we fix this guy. We'll pair on it.
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.
... aaaah children:transition
! Always mischievous, never caught.
* unified-desktop-gui: refactor: move currentProject to root data (#18834) chore: additional app shape cleanup (#18826) feat(useCollapsibleTree): adding support for building expandable and collapsible trees (#18860) feat: reconcile terminal command components (#18853) feat: setup keyboard shortcuts modal (#18864) feat: inline spec list header (#18863) feat: Runner spec list (#18821)
…e-data-clean-refactor * tgriesser/chore/e2e-data-clean: refactor: move currentProject to root data (#18834) chore: additional app shape cleanup (#18826) feat(useCollapsibleTree): adding support for building expandable and collapsible trees (#18860) feat: reconcile terminal command components (#18853) feat: setup keyboard shortcuts modal (#18864) feat: inline spec list header (#18863) feat: Runner spec list (#18821)
Created the flat view of specs list
Demo
Runner.Spec.List.mov
PR Tasks
cypress-documentation
?type definitions
?cypress.schema.json
?