Add toggle between logo and full title on click in details screen#2760
Add toggle between logo and full title on click in details screen#2760VyomVyas4765 wants to merge 3 commits intorecloudstream:masterfrom
Conversation
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
Great first commit, however do not add additional logic that conflicts with existing code. We already have a result_title textview, and a bindLogo function. Modify those instead of adding new code. Moreover, while your result_full_title_text might have nextFocus to navigate from, it can not be navigated to.
|
Thanks for the feedback! That makes sense, I’ll refactor it to use result_title and bindLogo, and also fix the navigation issue. |
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
Good improvements, however there are some visual errors that make this feature look weird.
- The UI is bugged on TV, the focus looks really weird. It should be a clear outline, not a white background.
- The UI is not clickable on TV with touch in the results view.
- As both views fight over the same layout location, the "animation" makes the UI move a lot. It is not seamless and has an affect on the entire layout tree.
- It is not serialized, saved or is in any shape persistent. Normally this would be fine, but bindLogo may get run several times on the same view, and the view will get recreated when switching e.g. tabs or recommendations. This makes the UI feel forgetful, inconsistent and even buggy when using it on the main page on TV. A local session cache on the url would fix this.
- Using
.isEnabledto store state will lead to inconsistent state, especially when using it in a recycleview.
| titleView.isVisible = false | ||
| titleView.isClickable = true | ||
| titleView.isFocusable = true | ||
| titleView.isFocusableInTouchMode = true |
There was a problem hiding this comment.
We normally do isFocusableInTouchMode = isLayout(TV) in CloudStream.
|
The details you gave actually helped a bunch, appreciate it. I've reworked the logic to fix the layout judder by dropping the animations, and hooked it up to a LruCache so the toggle state persists across the RecyclerView. I also dropped the .isEnabled hack to prevent state bleeding and cleaned up the TV focus outlines. Let me know if this looks better |
2 participants
Implements the feature for #2650 by adding a toggle on the logo to show the full title. Tapping the logo switches to the text title of the media and then tapping the title again brings the logo back. Nothing changes when there is no logo and the title itself is there. Tested and working as expected.