-
Notifications
You must be signed in to change notification settings - Fork 25
EVG-6867: Support searching by git hash on commit history #2073
Conversation
I used the feature in staging to find active and inactive commits and it works really well to center a commit. There are some issues around paginating backwards which limits utility since that's probably something that users will want to do after searching. We could merge this in as an experimental feature to see how users utilize jumping to a commit which could help with a future redesign. Since fixing the pagination issue is a major lift, it may be worth collecting analytics and partially meeting users' needs than not meeting them at all. This might be good to discuss at the UI roundtable. |
src/pages/commits/RenderCommit.tsx
Outdated
if (revision.trim() === "") { | ||
return false; | ||
} | ||
if (version && version.revision.includes(revision)) { |
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.
if (version && version.revision.includes(revision)) { | |
if (version?.revision.includes(revision)) { |
src/pages/commits/RenderCommit.tsx
Outdated
if ( | ||
rolledUpVersions && | ||
rolledUpVersions.some((v) => v.revision.includes(revision)) | ||
) { | ||
return true; | ||
} | ||
return false; |
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.
if ( | |
rolledUpVersions && | |
rolledUpVersions.some((v) => v.revision.includes(revision)) | |
) { | |
return true; | |
} | |
return false; | |
return rolledUpVersions && rolledUpVersions.some((v) => v.revision.includes(revision)) |
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.
I kept this one since I think it's easier to read, but I implemented your other feedback
@@ -70,7 +70,7 @@ export const CommitChart: React.FC<Props> = ({ | |||
const onToggleAccordion = ({ isVisible }) => | |||
Cookies.set(COMMIT_CHART_TYPE_VIEW_OPTIONS_ACCORDION, isVisible.toString()); | |||
|
|||
return hasError ? ( | |||
return noData ? ( |
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.
return noData ? ( | |
return !versions ? ( |
Passing run #13927 ↗︎
Details:
Review all test suite changes for PR #2073 ↗︎ |
evergreen retry |
EVG-6867
Description
This PR includes the frontend changes to make it possible to jump to a specific commit in the commit history.
Screenshots
commit_history.mov
Testing
search by git commit
block is new)Evergreen PR