Add size sorting to file list (#254)#334
Conversation
Add SIZE_ASC and SIZE_DESC options to the SortMethod enum so users can sort the file list by size (smallest-first or largest-first). Uses remoteSize with localSize fallback; null/zero sizes sort last. Includes Vitest tests for both directions, null handling, and tiebreaking by name. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds size-based sorting to the file list: new Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "File Options UI"
participant OptionsSvc as "ViewFileOptionsService"
participant SortSvc as "ViewFileSortService"
participant FileSvc as "ViewFileService"
UI->>OptionsSvc: user selects SortMethod (e.g. SIZE_ASC)
OptionsSvc-->>SortSvc: emits new options (sortMethod)
SortSvc->>SortSvc: compute comparator (getEffectiveSize + comparator)
SortSvc->>FileSvc: setComparator(comparator)
FileSvc->>FileSvc: apply comparator when sorting file list
FileSvc-->>UI: updated sorted file list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/angular/src/app/pages/files/file-options.component.html`:
- Around line 145-156: The new <img> icons inside the
file-options.component.html conditional blocks for SortMethod.SIZE_ASC and
SortMethod.SIZE_DESC (and the similar icons later at lines ~181–187) are missing
alt attributes; update each <img> (e.g., those with class names sort-asc and
sort-desc) to be decorative by adding alt="" and aria-hidden="true" so assistive
tech ignores them while the visible <span class="text"> remains the accessible
label.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2b58a26d-9c69-4d6e-a345-cbb532882650
📒 Files selected for processing (4)
src/angular/src/app/models/view-file-options.tssrc/angular/src/app/pages/files/file-options.component.htmlsrc/angular/src/app/services/files/view-file-sort.service.spec.tssrc/angular/src/app/services/files/view-file-sort.service.ts
| @if ((options | async)?.sortMethod === SortMethod.SIZE_ASC) { | ||
| <div class="sel-item"> | ||
| <div class="icon"><img src="assets/icons/sort-asc.svg" class="sort-asc" /></div> | ||
| <span class="text">Size S→L</span> | ||
| </div> | ||
| } | ||
| @if ((options | async)?.sortMethod === SortMethod.SIZE_DESC) { | ||
| <div class="sel-item"> | ||
| <div class="icon"><img src="assets/icons/sort-desc.svg" class="sort-desc" /></div> | ||
| <span class="text">Size L→S</span> | ||
| </div> | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify any Angular template img tags still missing alt attributes
rg -nP --type-add 'html:*.html' --type=html '<img(?![^>]*\balt=)[^>]*>' src/angular/srcRepository: nitrobass24/seedsync
Length of output: 4209
Add alt attributes to the new sort icons for accessibility.
Lines 147, 153, 181, and 187 introduce <img> elements without alt. Since visible text already labels each option, use decorative alt text (alt="") and hide icons from assistive tech with aria-hidden="true".
Proposed fix
- <div class="icon"><img src="assets/icons/sort-asc.svg" class="sort-asc" /></div>
+ <div class="icon"><img src="assets/icons/sort-asc.svg" class="sort-asc" alt="" aria-hidden="true" /></div>
- <div class="icon"><img src="assets/icons/sort-desc.svg" class="sort-desc" /></div>
+ <div class="icon"><img src="assets/icons/sort-desc.svg" class="sort-desc" alt="" aria-hidden="true" /></div>(Also applies to lines 181–187)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @if ((options | async)?.sortMethod === SortMethod.SIZE_ASC) { | |
| <div class="sel-item"> | |
| <div class="icon"><img src="assets/icons/sort-asc.svg" class="sort-asc" /></div> | |
| <span class="text">Size S→L</span> | |
| </div> | |
| } | |
| @if ((options | async)?.sortMethod === SortMethod.SIZE_DESC) { | |
| <div class="sel-item"> | |
| <div class="icon"><img src="assets/icons/sort-desc.svg" class="sort-desc" /></div> | |
| <span class="text">Size L→S</span> | |
| </div> | |
| } | |
| `@if` ((options | async)?.sortMethod === SortMethod.SIZE_ASC) { | |
| <div class="sel-item"> | |
| <div class="icon"><img src="assets/icons/sort-asc.svg" class="sort-asc" alt="" aria-hidden="true" /></div> | |
| <span class="text">Size S→L</span> | |
| </div> | |
| } | |
| `@if` ((options | async)?.sortMethod === SortMethod.SIZE_DESC) { | |
| <div class="sel-item"> | |
| <div class="icon"><img src="assets/icons/sort-desc.svg" class="sort-desc" alt="" aria-hidden="true" /></div> | |
| <span class="text">Size L→S</span> | |
| </div> | |
| } |
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 147-147: An alt attribute must be present on elements.
(alt-require)
[warning] 153-153: An alt attribute must be present on elements.
(alt-require)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/angular/src/app/pages/files/file-options.component.html` around lines 145
- 156, The new <img> icons inside the file-options.component.html conditional
blocks for SortMethod.SIZE_ASC and SortMethod.SIZE_DESC (and the similar icons
later at lines ~181–187) are missing alt attributes; update each <img> (e.g.,
those with class names sort-asc and sort-desc) to be decorative by adding alt=""
and aria-hidden="true" so assistive tech ignores them while the visible <span
class="text"> remains the accessible label.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
1 participant
Summary
SIZE_ASCandSIZE_DESCtoSortMethodenumremoteSize, fall back tolocalSize, nulls sort last, ties break by nameTest plan
Closes #254
🤖 Generated with Claude Code
Summary by CodeRabbit