-
-
Notifications
You must be signed in to change notification settings - Fork 522
feat: update ui components to material 3 expressive #7444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In terms of design, I think we should move the forward/backward buttons to the position of the next/prev buttons and move the latter somewhere else (perhaps next to the timeline?). This would:
- give the entire row more breathing space so it doesn't look so cramped
- avoid the current issue where the next/prev buttons look far more important
- bring the design closer to that showcased
Yeah, I agree. I'm just not too sure how to implement this because we have to set the time that we seek by dynamically (there's a preference to change it between 5s, 10s, 30s, ...). Currently we're using a FrameLayout for this, and we can't nest other layouts inside a button afaik. So we probably need to subclass button? 🤔 |
|
Okay, I decided to just drop the text, it's not that useful and good-looking anyways. |
e269dd6 to
56faf4d
Compare
XRecorder_20250525_01.mp4 |
|
Can't reproduce 1 and 3. |
Have you checked the latest CI version? / 3 hours ago
XRecorder_20250525_04.mp4
It seems that "material you" update was a bit problematic. My device |
This PR only includes user interface issues, anything like "autoplay not working" is not related to these changes here and should be discussed in a different issue. |
I found the reason for this, I will open a new issue. ( for 2 issue). There is a problem in the debug version. It would be great if 4 bugs were fixed. |
|
how to become tester? where can i get this build |
While logged in go to the checks tab -> CI under this PR, e.g. |
|
Some button animations are still a bit buggy, that's because we use the alpha version of Material 3 Expressive and it will probably get better with the next updates of the Material 3 Expressive library. |
Can the 4 bugs I mentioned above be fixed? The last CI failed Off topic: You should double check the autoplay bug with the autoplay countdown. |
Probably? Git patches that I can apply in this PR are welcome.
Yeah, the GitHub server is timeouting. Nothing we can do. I retriggered it, seemed to work now. |
app/src/main/java/com/github/libretube/ui/fragments/WatchHistoryFragment.kt
Show resolved
Hide resolved
Issues 2,3,4 can be repeated on many devices |
|
CI failed. I would like to test |
|
I'm currently working on a different PR to migrate to SDK 35, which is however complicated because it forces edge to edge and thus many changes are required. This PR depends on the SDK35 changes. |
|
@Bnyro So once sdk35 PR is done , is it possible to generate a new build here to testing purposes? Thanks |
649b510 to
4fa6b9b
Compare
Fixes an issue, where part of the text of the bottom-bar could be hidden.
|
Ready to merge from my POV. There are still some small adjustments needed before the release, but overall this should be fine now. The home page feels still a bit unfinished, but I'm not sure how to change the look of the "featured" and "continue watching" section. I redesigned the trends, bookmarks and playlists category. @FineFindus can you please have a final look at the latest two commits? |
|
#7406 will need to be rebased and merged after this PR. |
This comment has been minimized.
This comment has been minimized.
|
The audio view is broken when switching from PiP to audio player. This is actually the case in the debug version as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The player still uses the old loading indicators (I have 0194733, if you want to try and cherry-pick it :))
- As noted in a previous comment of mine, some of the texts are barely to not at all readable.
The home page feels still a bit unfinished
We should add chevrons the section titles, to indicate that they are clickable.
Neither of them are blockers for merging this though
| <com.google.android.material.button.MaterialButton | ||
| android:id="@+id/trending_category" | ||
| style="?materialIconButtonOutlinedStyle" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center" | ||
| app:icon="@drawable/ic_frame" /> | ||
|
|
||
| <com.google.android.material.button.MaterialButton | ||
| android:id="@+id/trending_region" | ||
| style="?materialIconButtonFilledTonalStyle" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center" | ||
| app:icon="@drawable/ic_region" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these should be moved to the settings as they are preference that are unlikely to be changed often, but that's not a blocker for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I thought it might be handy to be able to browse through the trends of different countries quickly, but perhaps it makes sense to move the region selector to the trending page directly?
Previously, by having it in the settings page, it perhaps wasn't that self-explaining to users what that region is used for (i.e. search results are not influenced by that setting, which might have been confusing, ...)
Yeah, I didn't migrate them at first because I thought it might look weird because all other video players also use the spinning circle. Just tested, looks fine, so we'll change them too :)
I'd appreciate help with that in a follow-up PR as I don't have the QPR1 Android version on my phone yet.
Good point, added! |
Co-authored-by: FineFindus <[email protected]>

ref https://m3.material.io/blog/building-with-m3-expressive
The audio player so far:

For other changes fragments, please see the commit history of this PR.
Slightly related to the changes here: #7406
TODO