feat(diff): Add search and extension filter to diff sidebar#37068
feat(diff): Add search and extension filter to diff sidebar#37068McMichalK wants to merge 46 commits into
Conversation
There is new button in pull request diff template, which opens dropbox containing list of all unique extensions that the diff contains. List is created with checkmarks that can be checked/unchecked to filter out files with specific extension. There is also "select all" and "deselect all" buttons. To apply the change and filter out the files user have to click on "Apply" button. If any extensions were filtered out, then button which opens the dropbox is outlined to signal some files were filtered out.
…FileExtensionFilter
GiteaBot
added
the
lgtm/need 2
This comment was marked as resolved.
This comment was marked as resolved.
|
Can we also move the filter to the left, above the files and also add the search bar? |
I think the right side is already very full |
My thinking was: there is already one feature filtering the diff - filtering by commits, so I've added similar feature nearby to let user have those side by side.
Also, as a daily gitea user, which is doing couple PR reviews per day, I do not agree that right side is very crowded right now, because everything is well placed. I'm open to discussion tho ;) |
…ng for loading more files and general code cleaning
|
In recent changes I've fixed the code for those issues:
And I also added the search box, as suggested. I did not changed the position of the button, waiting for more voices for this change. |
@silverwind whats your opinion on that? |
bircni
left a comment
bircni
left a comment
There was a problem hiding this comment.
Please do a proper pass over the AI-generated output before requesting review.
GiteaBot
added
lgtm/blocked
GiteaBot
added
lgtm/need 1
|
Could someone else review, please? |
ci failure are related. |
@lunny I'm not sure what's wrong, because I managed to run those tests locally and it's passing no problem, so I'm not sure what to do about this? |
Diff-boxes are now in separate div, so div with "no file matched" text is in separate place in parent div
@lunny @puni9869 Thank you for pointing me the link, I managed to fix the failing tests now and sorry for trouble. |
|
Anyone else would like to take a look? |
|
@wxiaoguang @bircni Hi, I would be most grateful if you could give this PR a look and one more approval, I've invested quite some time in this so it would be very cool to see it merged. |
|
Finally had the time to try Fable so here you go Review — file extension filterNice feature overall! I ran a deeper review over the branch; 4 correctness issues in the new filtering logic and 4 smaller cleanups. Details below, roughly ordered by severity. 🐛 Correctness
const matched = matches(newName) || (oldName !== newName && matches(oldName));The file boxes match against
On a paginated diff, a filter that only matches not-yet-loaded files gives
🧹 Cleanups
Each character triggers
The only childList mutation of
Two independent implementations of the same Go-style placeholder handling means a fix to one silently misses the other. Suggest one shared helper next to
In particular Review assisted by Claude Code (Fable 5). |
GiteaBot
added
lgtm/done
| data-file-extensions="{{ctx.Locale.Tr "repo.diff.file_extensions"}}" | ||
| data-no-file-extension="{{ctx.Locale.Tr "repo.diff.no_file_extension"}}" | ||
| data-select-all-file-extensions="{{ctx.Locale.Tr "repo.diff.select_all_file_extensions"}}" | ||
| data-select-none-file-extensions="{{ctx.Locale.Tr "repo.diff.select_none_file_extensions"}}" |
There was a problem hiding this comment.
The locale related attributes should have "data-text-" or "data-locale-" prefix to distinguish them from other attributes.
| const el = document.querySelector<HTMLElement>('#diff-file-tree')!; | ||
|
|
||
| const filterFilesPlaceholder = el.getAttribute('data-text-filter-files')!; | ||
| const filterFilesNoResults = el.getAttribute('data-text-filter-files-no-results')!; | ||
| const filterFilesClearLabel = el.getAttribute('data-text-filter-files-clear')!; |
There was a problem hiding this comment.
Should avoid making the Vue component query its parent.
See other Vue component, the caller should prepare the props (locale attributes) for it.
| el.textContent = fillTemplate(el.getAttribute('data-text-loaded') ?? '', loadedFiles, totalFiles); | ||
| } | ||
|
|
||
| function updateShowMoreButton(matchingBelow: number) { | ||
| const btn = document.querySelector('#diff-show-more-files'); | ||
| if (!btn) return; | ||
| if (matchingBelow > 0) { | ||
| btn.textContent = fillTemplate(btn.getAttribute('data-text-matching') ?? '', matchingBelow); | ||
| } else { | ||
| btn.textContent = btn.getAttribute('data-text-default') ?? btn.textContent; | ||
| } |
There was a problem hiding this comment.
Why you expect that these attributes wouldn't exist ........
|
Is it expected that when I click "Select none", then nothing is listed? 
|
|
Also, some reviews from #37068 (comment) are still valid |
Add file extension filter for pull request diffs
Introduces a new dropdown filter in the PR diff toolbar that allows reviewers to show/hide files based on their extensions. This helps focus on specific file types during code review (e.g., viewing only
.tsfiles when reviewing TypeScript changes, or hiding.test.jsfiles when reviewing implementation).Closes: Feature of file filter in PR page #27256
Usage
How it works
.tw-hiddenCSS class to non-matching files, instantly hiding themKeyboard Navigation
For users who prefer keyboard navigation:
Technical Details
DiffFileExtensionFilter.vue) following the same pattern as the existing commit filterTesting
Screenshots
Filter by extension dropdown:
Note: This implementation uses AI assistance (GitHub Copilot). All code has been reviewed and tested against Gitea's contribution guidelines and passes all linting checks.