Use vector drawables instead of PNGs for material icons by Stypox · Pull Request #3271 · TeamNewPipe/NewPipe

Stypox · 2020-03-25T15:26:20Z

What is it?

Bug fix
Feature
Enhancement under the hood

Long description of the changes in your PR

For all manually-created images PNG have been kept. Debug apk size decreased by 232,030 bytes.

Rename all icon attrs to have a ic_ prefix
Always use _24dp icons, because there is no real difference, since they are vector drawables
Always use the original name found on material.io for icon drawables, as to not create confusion and possibly duplicates. Icon names can still be different from real drawable names, though I have made some of them compliant to this or maybe more meaningul.
Use standard icons for expand_more and expand_less instead of triangles
Use play_button_outline instead of custom PNG as play button in VideoDetailFragment (questionable, as there is no shadow anymore)
Replace the search-insert-icon (on the button that is right to every search suggestion, and when pressed replaces what is in the search field with the value of that suggestion), that was an arrow pointing up-left and didn't have a meaning, with a custom hand-made icon (ic_search_add)
Keep ic_close and ic_replay as PNGs: needed for notifications in RemoteViews

Other changes:

Remove duplicate getIconByAttr() in ThemeHelper (use resolveResourceIdFromAttr() instead)
Improve the VideoDetailFragment layout by a little by forcing the "expand description" icon to always stay in the same place even after expanding. I tested the new layout on various screens and it always looked ok.
Update AndroidX to 1.1.0 to fix icon crashes in preferences on API 19: use androidx.preference:preference instead of androidx.legacy:legacy-preference-v14
Remove legacy libraries (androidx.legacy:legacy-support-v4): this made no effect, and allowed me not to use androidx.appcompat.widget.* everywhere, tested on various Android versions
Fix SelectChannelFragment and SelectKioskFragment (those used to set main screen tabs): in API 19 they had a duplicate title always empty.
Fix pause icon used instead of play when seeking in a paused popup player

Fixes the following issue(s)

Fixes New drawables and icons #1199

Testing apk

Tested thoroughly on API 19 (Galaxy Nexus (small screen) and Pixel 3 XL (big screen) emulators on Android 4.4), on API 24 (native on Huawei P9 Lite with Android 7.0), on API 29 (Pixel 3 XL emulator on Android 10). Though I'm not entirely sure I have explored every single piece of the app, so I'd like @domiuns and @Anotherlife to test it, too, on Android 4.4 ;-)
Debug apk: app-debug_2.zip

Agreement

I carefully read the contribution guidelines and agree to them.

app/src/main/java/org/schabi/newpipe/player/ServicePlayerActivity.java

VGkav · 2020-04-01T22:35:40Z

Oh, no, it crashed when I tried to fast forward with double-tapping. While in fullscreen.

https://pastebin.com/QmfEPdsf

Stypox · 2020-04-02T06:34:47Z

@Anotherlife fixed with the latest commit, could you test this new apk? app-debug_1.zip

VGkav · 2020-04-02T07:55:44Z

Seems good now :D

VGkav · 2020-04-02T07:56:45Z

Wait, there is no icon to maximise a popup window! The control is there and it works but there is no icon visible.

Stypox · 2020-04-02T08:06:00Z

I fixed it in the last commit, but didn't upload a new apk due to connection problems 😅

Stypox · 2020-04-02T08:21:03Z

Here it is: app-debug_2.zip

Stypox · 2020-04-15T09:49:44Z

@TobiGr @wb9688 can I add this to the 0.19.3 milestone? It should be reviewed&merged soon imo, because there are many changed files causing conflicts and at the end of the day it is not really a breaking change.

wb9688 · 2020-04-15T09:57:55Z

@Stypox: We don't want to add anything new to the v0.19.3 milestone. I think you should add it to the v0.20.0 milestone.

Stypox · 2020-05-06T19:16:48Z

I just rebased

Stypox · 2020-05-20T18:54:58Z

@Anotherlife @domiuns @mqus @jojoxl01 @ask6155 @skil3z could you test this apk on your Android 4.4 devices? In particular you sould verify if all icons are visible. Thank you in advance :-)
app-debug.zip

VGkav · 2020-05-20T20:34:49Z

Looks good to me, I didn't see any icons missing. Unless I didn't see them because they were missing ... badum tisshhh

TobiGr · 2020-05-20T20:40:28Z

Ah. just saw one thing: Please undo the icon change in the navigation drawer. According to material.io a:small_red_triangle_down: should be used and not a v.

Stypox · 2020-05-21T07:11:02Z

Oh sorry, didn't notice that. I replaced the expand icon with a drop one in the latest commit:
app-debug_1.zip

For all manually-created images PNG have been kept. - rename all icon attrs to have a `ic_` prefix - always use `_24dp` icons, because there is no real difference, since they are vector drawables - always use the original name found on material.io for icon drawables, as to not create confusion and possibly duplicates. Icon names can still be different from real drawable names, though I have made some of them compliant to this or maybe more meaningul. - remove duplicate `getIconByAttr()` in ThemeHelper (use `resolveResourceIdFromAttr()` - use standard icons for `expand_more` and `expand_less` instead of triangles - use `play_button_outline` instead of custom PNG as play button in VideoDetailFragment (questionable, as there is no shadow anymore)

To fix crashes on API 19

The other icons used in notifications are taken from exoplayer's ones: `@drawable/exo_controls_*`

Also remove legacy libraries Use `androidx.preference:preference` instead of `androidx.legacy:legacy-preference-v14` and remove `androidx.legacy:legacy-support-v4

Also use `setBackgroundResource` to automatically obtain PNG drawables (from exoplayer)

Also replace all tabs with 4 spaces

As per the material guidelines

TobiGr

Thanks!

jojoxl01 · 2020-05-22T00:59:43Z

At present, no problems have been found and everything is normal. Thank you!

…

------------------------------ 在 2020年5月21日星期四，Stypox <notifications@github.com> 写道：

Oh sorry, didn't notice that. I replaced the expand icon with a drop one in the latest commit: app-debug_1.zip <https://github.com/TeamNewPipe/NewPipe/files/4660987/app-debug_1.zip> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3271 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHZ5MWIQS5IIC3ORM3OFBMLRSTHZHANCNFSM4LTRDG3Q> .

B0pol reviewed Mar 25, 2020

View reviewed changes

app/src/main/java/org/schabi/newpipe/player/ServicePlayerActivity.java Outdated Show resolved Hide resolved

Stypox force-pushed the icons branch from 35f6dff to e12f4b3 Compare March 25, 2020 19:34

Stypox added feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface labels Mar 26, 2020

mauriciocolli mentioned this pull request Mar 26, 2020

Improvements for the drawer header layout #3259

Merged

Stypox force-pushed the icons branch from e12f4b3 to 63e3cd2 Compare March 31, 2020 10:48

Stypox mentioned this pull request Apr 1, 2020

Crash when trying to switch to my currently playing playlist #3320

Closed

Stypox force-pushed the icons branch 4 times, most recently from 07efdea to 7b91ba1 Compare April 1, 2020 20:34

Stypox marked this pull request as ready for review April 1, 2020 20:47

Stypox force-pushed the icons branch 2 times, most recently from 4bd8822 to 1c03741 Compare April 10, 2020 20:38

wb9688 mentioned this pull request Apr 12, 2020

Compress PNG images losslessly via zopflipng #2614

Closed

1 task

Stypox modified the milestones: 0.20.0, 0.19.4 Apr 15, 2020

Stypox force-pushed the icons branch from 1c03741 to ad4cfc9 Compare May 6, 2020 19:14

Stypox force-pushed the icons branch 2 times, most recently from 1aef2f7 to 9d338d0 Compare May 9, 2020 14:19

Stypox added 16 commits May 21, 2020 15:39

Use app:srcCompat and derivatives intead of android: ones

3dcd246

To fix crashes on API 19

Rename ic_hot in ic_kiosk_hot and improve getKioskIcon()

c27a26c

Fix MainFragment tab icons did not follow theme color

e0e4f6d

Fix background player queue crashing on opening

d7f6101

Readd ic_close and ic_replay PNGs: needed in notifications

631dfee

The other icons used in notifications are taken from exoplayer's ones: `@drawable/exo_controls_*`

Update AndroidX to fix icon crashes in preferences on API 19

b5c60d2

Also remove legacy libraries Use `androidx.preference:preference` instead of `androidx.legacy:legacy-preference-v14` and remove `androidx.legacy:legacy-support-v4

Fix choice dialogs on API 19 by manually getting drawable

b575046

Fix additional empty title on tab selection fragments in API 19

899f69d

Replace hardcoded white drawable with themed one

158727e

Fix pause used instead of play in paused popup player when seeking

883bcc7

Also use `setBackgroundResource` to automatically obtain PNG drawables (from exoplayer)

Use AppCompatResources instead of ContextCompat

e3e2028

Fix invisible fullscreen button in popup player on API 19

05ab54c

Fix checkstyle issues

9e57195

Also replace all tabs with 4 spaces

Add shadow behind play icon in video detail fragment

bbd0df0

Use drop down/up instead of expand icons in drawer

927a1d5

As per the material guidelines

Stypox force-pushed the icons branch from e12f9bd to 927a1d5 Compare May 21, 2020 13:39

TobiGr approved these changes May 21, 2020

View reviewed changes

TobiGr merged commit 176d57b into TeamNewPipe:dev May 21, 2020

wb9688 mentioned this pull request May 25, 2020

Fix toolbar icon and text color in white theme #3482

Closed

5 tasks

This was referenced May 29, 2020

Release 0.19.4 #3669

Merged

Release 0.19.4 #3670

Closed

blackbox87 mentioned this pull request Jun 12, 2020

Layout fixes TeamNewPipe/NewPipe-legacy#31

Merged

5 tasks

Stypox mentioned this pull request Jul 21, 2020

Expandable player with unified UI #2907

Merged

Stypox deleted the icons branch August 4, 2022 09:47

Uh oh!

Conversation

Stypox commented Mar 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is it?

Long description of the changes in your PR

Fixes the following issue(s)

Testing apk

Agreement

Uh oh!

Uh oh!

VGkav commented Apr 1, 2020

Uh oh!

Stypox commented Apr 2, 2020

Uh oh!

VGkav commented Apr 2, 2020

Uh oh!

VGkav commented Apr 2, 2020

Uh oh!

Stypox commented Apr 2, 2020

Uh oh!

Stypox commented Apr 2, 2020

Uh oh!

Stypox commented Apr 15, 2020

Uh oh!

wb9688 commented Apr 15, 2020

Uh oh!

Stypox commented May 6, 2020

Uh oh!

Stypox commented May 20, 2020

Uh oh!

VGkav commented May 20, 2020

Uh oh!

TobiGr commented May 20, 2020

Uh oh!

Stypox commented May 21, 2020

Uh oh!

TobiGr left a comment

Choose a reason for hiding this comment

Uh oh!

jojoxl01 commented May 22, 2020 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Stypox commented Mar 25, 2020 •

edited

Loading