Improve subtitle selection UX: Move "No Subtitles" option to bottom#2523
Improve subtitle selection UX: Move "No Subtitles" option to bottom#2523hrisabhy wants to merge 1 commit intorecloudstream:masterfrom
Conversation
|
The idea to move "No subtitles" to the bottom makes sense from a consistency standpoint, but moving it below the '+ ...' footers is inconsistent with other subtitles. If you want this implemented, the "No subtitles" option would need to be above them. |
|
Hi @fire-light42, I've updated the PR based on your feedback. Changes made:
The footer ordering is now:
|
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
It looks like you are using AI to assist you in creating this pull request. Please read our AI policy and mark your pull request accordingly.
| } | ||
|
|
||
| val noSubtitlesFooter: TextView = | ||
| layoutInflater.inflate(R.layout.sort_bottom_single_choice, null) as TextView |
There was a problem hiding this comment.
Why are you creating a whole new view? We just need simple reordering of what we already have. This creates unnecessary complexity and change of existing logic.
This pull request should be roughly 7 lines changed, not 46.
There was a problem hiding this comment.
Thanks for pointing this out. You were right my earlier change was unnecessarily complex.
I have pushed an update keeping the change minimal and avoided touching existing logic.
Also about the AI part I did initially take some assistance while drafting but I reworked the changes myself to match the project style. I will make sure to follow the AI policy properly and mark it next time.
Let me know if anything still needs adjustment.
|
|
||
| // Cheeky way of getting the view at that position to click it | ||
| // to avoid keeping track of the various footers. | ||
| // getChildAt() gives null :( |
There was a problem hiding this comment.
You are removing valuable comments for no reason. Do not change existing code or comments without justification.
There was a problem hiding this comment.
Got it 👍 I have restored the important comments and avoided changing existing ones unless necessary.
- Moved 'No Subtitles' from top to bottom of adapter - Adjusted subtitle index calculations (removed +1 offset) - Changed condition from <= 0 to >= size for detecting no subtitle selection
hrisabhy
force-pushed
the
fix/subtitle-toggle-ux
branch
from
57f107e to
eab2b07
Compare
2 participants
Problem
The "No Subtitles" option was hidden at the top of the subtitle list, requiring users to scroll up to discover it. This was not intuitive, especially for new users who expected options to be visible or at the bottom.
Solution
sort_bottom_footer_add_choicetosort_bottom_single_choicelayoutChanges
Files modified:
app/src/main/java/com/lagradost/cloudstream3/ui/player/GeneratorPlayer.ktsubtitleGroupIndexStartcalculation (removed +1 offset)app/src/main/res/layout/player_select_source_and_subs.xmlTesting
Tested on Android device with multiple subtitle sources. Verified:
Demo
Before
"No Subtitles" hidden at top, requiring upward scroll
After
"No Subtitles" visible at bottom, scroll indicators show more content
Related
Improves UX for subtitle management without requiring new features or breaking changes.