fix: Update viewFile toggle in commit detail page #3623
Conversation
Bundle ReportChanges will decrease total bundle size by 435 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3623 +/- ##
==========================================
- Coverage 98.98% 98.94% -0.04%
==========================================
Files 811 811
Lines 14543 14556 +13
Branches 4127 4134 +7
==========================================
+ Hits 14395 14403 +8
- Misses 141 146 +5
Partials 7 7
... and 1 file with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3623 +/- ##
==========================================
- Coverage 98.98% 98.94% -0.04%
==========================================
Files 811 811
Lines 14543 14556 +13
Branches 4127 4127
==========================================
+ Hits 14395 14403 +8
- Misses 141 146 +5
Partials 7 7
... and 1 file with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Bundle ReportChanges will decrease total bundle size by 435 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3623 +/- ##
==========================================
- Coverage 98.98% 98.94% -0.04%
==========================================
Files 811 811
Lines 14543 14556 +13
Branches 4120 4127 +7
==========================================
+ Hits 14395 14403 +8
- Misses 141 146 +5
Partials 7 7
... and 1 file with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #3623 +/- ##
==========================================
- Coverage 98.98% 98.94% -0.04%
==========================================
Files 811 811
Lines 14543 14556 +13
Branches 4127 4127
==========================================
+ Hits 14395 14403 +8
- Misses 141 146 +5
Partials 7 7
... and 1 file with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
suejung-sentry
force-pushed
the
sshin/2399
branch
from
ec2ce64 to
0f8b501
Compare
suejung-sentry
force-pushed
the
sshin/2399
branch
from
0f8b501 to
497eacd
Compare
| ) : null} | ||
| {isDeletedFile ? ( |
There was a problem hiding this comment.
very nice that we can delete this conditional logic now
| })} | ||
| data-testid="file-diff-expand" | ||
| onClick={() => !isDeletedFile && row.toggleExpanded()} | ||
| {...(!isDeletedFile && { 'data-highlight-row': 'onHover' })} |
There was a problem hiding this comment.
jw, does 'data-highlight-row': !isDeletedFile ? 'onHover' : undefined (or w/e the default is) work?
It's a little easier for me to parse that way but idk maybe that's worse
There was a problem hiding this comment.
Nice yeah I like that better, too! Updated!
| @@ -297,7 +268,7 @@ export default function FilesChangedTable() { | |||
| getCoreRowModel: getCoreRowModel(), | |||
| getSortedRowModel: getSortedRowModel(), | |||
| getExpandedRowModel: getExpandedRowModel(), | |||
| getRowCanExpand: () => true, | |||
| getRowCanExpand: (row) => row.original?.headCoverage !== null, // deleted files are not expandable | |||
There was a problem hiding this comment.
Is this check used elsewhere (outside of these tables I mean)? Jw how we know this is the item to key off of when making a determination of if the file is deleted or not
There was a problem hiding this comment.
Oh good question. I pulled that from an existing usage from this table, but I get what you mean, maybe we should have this be a more inherent property of the file in api than having the UI make that designation here adhoc. And something more obvious than headCoverage being null (which can in theory happen for more reasons, not just due to a deleted file). I'll have to think about that one but in the meantime may ship this work independently
There was a problem hiding this comment.
yep totally fair! I'll keep this in the back of my head too in case I stumble upon a more straight-forward abstraction of this info at some point
Update the behavior of a file row so clicking anywhere in the row opens the toggle instead of before where clicking the filename takes you to the Files Explorer View but clicking the carat opens that file's contents.
BEFORE
Fixes the below tabs that all had the "confusing" behavior:
Notes:
Closes codecov/engineering-team#2399
AFTER
example links:
commit page, direct files - https://preview-pr-3623.codecov.dev/gh/codecov/gazebo/pull/3623
commit page, indirect files - https://preview-pr-3623.codecov.dev/gh/codecov/gazebo/pull/3623/indirect-changes
pull page, direct files - https://preview-pr-3623.codecov.dev/gh/codecov/gazebo/commit/324ff6c7c411ddace00d0b8b91201d13a4707962
pull page, indirect files - https://preview-pr-3623.codecov.dev/gh/codecov/gazebo/commit/56cde4f21090d50d450c4d77091f8d83e2a03221/indirect-changes
deleted file example
commit - http://preview-pr-3623.codecov.dev/gh/codecov/gazebo/commit/aa4c740b48099f636bf3ea61e359b52ef56d4d7a
pull - http://preview-pr-3623.codecov.dev/gh/codecov/gazebo/pull/3601