Skip to content

Add right padding for alphabet scrollbar#162

Merged
phanan merged 6 commits into
masterfrom
feat/scrollbar-padding
Mar 30, 2026
Merged

Add right padding for alphabet scrollbar#162
phanan merged 6 commits into
masterfrom
feat/scrollbar-padding

Conversation

@phanan
Copy link
Copy Markdown
Member

@phanan phanan commented Mar 29, 2026

Summary

  • Add rightPadding parameter to PlayableListHeader and SliverPlayableList
  • When the alphabet scrollbar is visible, pass alphabetScrollbarWidth as right padding to prevent content from being obscured
  • Applied to all 5 screens with alphabet scrollbar: songs, favorites, playlist details, artist details, genre details

Test plan

  • Open a screen with enough songs sorted by title — verify the alphabet scrollbar doesn't overlap the song rows or Play/Shuffle buttons
  • Switch to a sort that hides the scrollbar — verify padding goes away
  • Run flutter test

Summary by CodeRabbit

  • Bug Fixes
    • Fixed content alignment and spacing when the alphabet scrollbar is displayed to prevent overlap with list items.
    • Ensured consistent layout and padding across all playable-list screens so headers and rows align with the optional scrollbar.
    • Reduced reserved scrollbar space to improve visual balance when the scrollbar is shown.

…r is visible

Prevents song rows and Play/Shuffle/Search controls from being
obscured by the alphabet scrollbar overlay.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 29, 2026

📝 Walkthrough

Walkthrough

Compute a single showScrollbar boolean per screen via AlphabetScrollbar.shouldShow(...), pass conditional rightPadding (now using 0.75*alphabetScrollbarWidth when shown) into list header and sliver list widgets, and render the AlphabetScrollbar based on that flag. Widgets gained rightPadding params.

Changes

Cohort / File(s) Summary
Widget API & Layout
lib/ui/widgets/playable_list_header.dart, lib/ui/widgets/sliver_playable_list.dart
Added rightPadding (default 0) to PlayableListHeader and SliverPlayableList. PlayableListHeader uses right: 16 + rightPadding; SliverPlayableList now wraps its SliverList in SliverPadding using EdgeInsets.only(right: rightPadding).
Song header param
lib/ui/screens/songs.dart
Added public rightPadding field/constructor param to SongListHeader and pass conditional rightPadding to header and SliverPlayableList.
Screen scrollbar unification
lib/ui/screens/artist_details.dart, lib/ui/screens/favorites.dart, lib/ui/screens/playlist_details.dart, lib/ui/screens/songs.dart
Each screen now computes showScrollbar once via AlphabetScrollbar.shouldShow(...), uses showScrollbar ? alphabetScrollbarWidth * 0.75 : 0 for rightPadding passed into headers and lists, and uses if (showScrollbar) to render AlphabetScrollbar.
Genre/Albums/Artists layout tweak
lib/ui/screens/genre_details.dart, lib/ui/screens/albums.dart, lib/ui/screens/artists.dart
Moved right-padding control into SliverPlayableList.rightPadding (genre) and changed padding amount from alphabetScrollbarWidth to alphabetScrollbarWidth * 0.75 (albums, artists). Removed external SliverPadding wrapper in genre details.
Small diffs / reuses
lib/ui/screens/... (other screens updated similarly)
Minor local refactors to reuse the computed showScrollbar and to apply the 0.75 width multiplier consistently across screens.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐇 I nudge the lists to hug the right,
One boolean keeps the padding light,
Scroll or not, rows line up neat—
A rabbit's hop, a tidy beat,
Widgets snug and UI bright. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding right padding to accommodate the alphabet scrollbar visibility. It is specific, concise, and clearly conveys the primary modification across multiple screens.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/scrollbar-padding

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

phanan added 5 commits March 30, 2026 12:36
SliverPadding shifted the entire list including dividers off-screen.
Per-row padding only insets the PlayableRow content while dividers
stay within bounds.
Wraps SliverList in SliverPadding so the entire list including
dividers is inset from the right, not individual rows.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@lib/ui/screens/songs.dart`:
- Line 156: The rightPadding currently uses 0.75 * alphabetScrollbarWidth which
can allow content to be obscured; update both occurrences to reserve the full
scrollbar width by changing the conditional to use alphabetScrollbarWidth (i.e.,
replace showScrollbar ? alphabetScrollbarWidth * 0.75 : 0 with showScrollbar ?
alphabetScrollbarWidth : 0) where rightPadding is set, referencing the
rightPadding property and the alphabetScrollbarWidth variable in songs.dart so
the layout fully avoids the scrollbar overlay.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 75c7f831-3dd1-4de6-9d13-fcad28173db5

📥 Commits

Reviewing files that changed from the base of the PR and between de6ab4f and c8b4f74.

📒 Files selected for processing (8)
  • lib/ui/screens/albums.dart
  • lib/ui/screens/artist_details.dart
  • lib/ui/screens/artists.dart
  • lib/ui/screens/favorites.dart
  • lib/ui/screens/genre_details.dart
  • lib/ui/screens/playlist_details.dart
  • lib/ui/screens/songs.dart
  • lib/ui/widgets/sliver_playable_list.dart
✅ Files skipped from review due to trivial changes (2)
  • lib/ui/screens/albums.dart
  • lib/ui/screens/artists.dart
🚧 Files skipped from review as they are similar to previous changes (5)
  • lib/ui/screens/favorites.dart
  • lib/ui/screens/genre_details.dart
  • lib/ui/widgets/sliver_playable_list.dart
  • lib/ui/screens/playlist_details.dart
  • lib/ui/screens/artist_details.dart

Comment thread lib/ui/screens/songs.dart
@phanan phanan merged commit 7dbbd32 into master Mar 30, 2026
2 checks passed
@phanan phanan deleted the feat/scrollbar-padding branch March 30, 2026 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant