fix: Make spec row clickable across entire width #22105

mike-plummer · 2022-06-03T22:19:30Z

Move click-sensitive row action wrapper to surround entire row
Add styles to highlight spec icon on hocus to match Figma
Small text highlight style change to match Figma

Closes Rows in Specs List should be clickable all the way across #21783

User facing changelog

Entire row in Specs List will respond to clicks
Styling updates to make current row in Specs List more obvious

Additional details

According to #21783 this was brought up a lot during v10 feedback testing. Current behavior only performs the row's action (toggle expansion for directories or navigate to spec for Spec rows) if you click directly on the row title item. This makes the entire row actionable by default.

Theoretically we may introduce row items that themselves want to respond to click events (opening a tooltip permanently, for example) - in that case the child can ensure the click event is not propagated upwards thus preventing the row from actioning.

Steps to test

Open Cypress app into a project with existing tests within directories
Navigate to Specs List
Validate that clicking directly on a Spec Name navigates, validate that clicking the row away from the spec name also navigates
Validate that keyboard navigation to Spec row is operational and that pressing enter triggers navigation
Validate that clicking directly on a directory name/icon toggles expansion, validate that clicking the row away from the directory name/icon also toggles expansion
Validate that keyboard navigation to Directory row is operational and that pressing enter toggles expansion
Validate that hovering over a Spec row adds indigo coloration to spec icon

How has the user experience changed?

Current

21783-current.mov

Updated

21783-new.mov

PR Tasks

Have tests been added/updated?
Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
[ na] Has a PR for user-facing changes been opened in cypress-documentation?
[ na] Have API changes been updated in the type definitions?

* Move click-sensitive row action wrapper to surround entire row * Add styles to highlight spec icon on hocus to match Figma * Small text highlight style change to match Figma

cypress-bot · 2022-06-03T22:19:33Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

cypress · 2022-06-06T15:46:59Z

Test summary

37553 • 0 • 454 • 0 • 3

Run details


Project	cypress
Status	Passed
Commit	`ad0ac9d`
Started	Jun 7, 2022 1:09 PM
Ended	Jun 7, 2022 1:25 PM
Duration	16:05 💡
OS	Linux Debian - 10.11
Browser	Multiple

View run in Cypress Dashboard ➡️

Flakiness

	next.cy.ts	1
1	Working with next-12.1.6 > should detect new spec
	cypress/proxy-logging.cy.ts	2
1	Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second
2	Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second

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

tbiethman · 2022-06-06T21:38:08Z

packages/app/src/specs/SpecsListRowItem.vue

+    <component
+      :is="isLeaf ? 'RouterLink' : 'div'"
+      class="h-full outline-none border-gray-50 ring-inset grid group focus:outline-transparent focus-within:ring-indigo-300 focus-within:ring-1 children:cursor-pointer"
+      :class="[isLeaf ? 'grid-cols-2' : 'grid-cols-1']"


I noticed that the directory rows render a little off-center:

I think that's because we flex this class to grid-cols-1 when it's not a leaf, but we continue to render the git info div on line 17 regardless.

That kinda has me thinking why we need the git-info slot here at all? Could we instead make git-info (and thus the grid classes) a concern of the SpecItem component and consolidate all the 'leaf'-related logic in one place?

I'm not sure if this is still the case but we used to have multiple views for this table, and I think some code was shared with the spec list on the runner page. That might explain some slots that don't seem to do much.

It's possible some old code is still hanging around from when we were still figuring the UI for this page out. Don't be afraid to delete code that seems weird/incorrect/not useful.

I don't think this is a dead/legacy code issue, rather just that we currently don't display anything in the "Last Updated" slot for directories from the newly-added Git Info data in this table. Since there's a non-zero chance we may decide to put something in that column for directories in the future I reverted the style change so the directory renders as expected and just has an empty second column

tbiethman · 2022-06-06T22:04:44Z

packages/app/src/specs/SpecsListRowItem.vue

+  <div data-cy="specs-list-row">
+    <component
+      :is="isLeaf ? 'RouterLink' : 'div'"
+      class="h-full outline-none border-gray-50 ring-inset grid group focus:outline-transparent focus-within:ring-indigo-300 focus-within:ring-1 children:cursor-pointer"


For the directory rows, it looks like there's some conflict between the transparent outline focus styles and the top border that acts as the divider:

You might just need to use focus:outline-none within the RowDirectory component and let these focus-within styles take precedent.

focus:outline-none works like a charm 💯

tbiethman · 2022-06-07T22:44:52Z

packages/app/src/specs/SpecsList.vue

@@ -96,7 +90,7 @@
              :style="{ paddingLeft: `${(row.data.depth - 2) * 10}px` }"
              :indexes="getDirIndexes(row.data)"
              :aria-controls="getIdIfDirectory(row)"
-              @click="row.data.toggle"
+              @click.stop="row.data.toggle"


Do we need both the click handler here and the one that emits the @toggleRow event from SpecsListRowItem? Seems like the SpecsListRowItem handler should catch clicks on the RowDirectory at this point.

tbiethman

Updates look good 👍 👍

* fix: Make spec row clickable across entire width * Move click-sensitive row action wrapper to surround entire row * Add styles to highlight spec icon on hocus to match Figma * Small text highlight style change to match Figma * Add missing data-cy selector * Add tests for row expansion behaviors * Fix directory row styling issues

fix: Make spec row clickable across entire width

430ceef

* Move click-sensitive row action wrapper to surround entire row * Add styles to highlight spec icon on hocus to match Figma * Small text highlight style change to match Figma

mike-plummer and others added 3 commits June 6, 2022 08:48

Add missing data-cy selector

7bcd321

Merge branch 'develop' into 21783-spec-rows-clickable-across-full-width

64dc697

Add tests for row expansion behaviors

3c94541

mike-plummer requested review from lmiller1990 and flotwig June 6, 2022 15:58

mike-plummer marked this pull request as ready for review June 6, 2022 15:58

mike-plummer self-assigned this Jun 6, 2022

tbiethman reviewed Jun 6, 2022

View reviewed changes

Fix directory row styling issues

ad0ac9d

flotwig approved these changes Jun 7, 2022

View reviewed changes

tbiethman reviewed Jun 7, 2022

View reviewed changes

tbiethman approved these changes Jun 7, 2022

View reviewed changes

lmiller1990 approved these changes Jun 8, 2022

View reviewed changes

lmiller1990 merged commit 0298b97 into develop Jun 8, 2022

lmiller1990 deleted the 21783-spec-rows-clickable-across-full-width branch June 8, 2022 05:35

cypress-bot bot mentioned this pull request Jun 8, 2022

Rows in Specs List should be clickable all the way across #21783

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Make spec row clickable across entire width #22105

fix: Make spec row clickable across entire width #22105

mike-plummer commented Jun 3, 2022 •

edited

Loading

cypress-bot bot commented Jun 3, 2022

cypress bot commented Jun 6, 2022 •

edited

Loading

tbiethman Jun 6, 2022 •

edited

Loading

lmiller1990 Jun 7, 2022

mike-plummer Jun 7, 2022

tbiethman Jun 6, 2022

mike-plummer Jun 7, 2022

tbiethman Jun 7, 2022

tbiethman left a comment

fix: Make spec row clickable across entire width #22105

fix: Make spec row clickable across entire width #22105

Conversation

mike-plummer commented Jun 3, 2022 • edited Loading

User facing changelog

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress-bot bot commented Jun 3, 2022

cypress bot commented Jun 6, 2022 • edited Loading

Test summary

Run details

Flakiness

tbiethman Jun 6, 2022 • edited Loading

Choose a reason for hiding this comment

lmiller1990 Jun 7, 2022

Choose a reason for hiding this comment

mike-plummer Jun 7, 2022

Choose a reason for hiding this comment

tbiethman Jun 6, 2022

Choose a reason for hiding this comment

mike-plummer Jun 7, 2022

Choose a reason for hiding this comment

tbiethman Jun 7, 2022

Choose a reason for hiding this comment

tbiethman left a comment

Choose a reason for hiding this comment

mike-plummer commented Jun 3, 2022 •

edited

Loading

cypress bot commented Jun 6, 2022 •

edited

Loading

tbiethman Jun 6, 2022 •

edited

Loading