Skip to content
This repository has been archived by the owner on Jun 14, 2022. It is now read-only.

Consider using an upstream icon package #370

Closed
xM8WVqaG opened this issue Jul 31, 2019 · 22 comments · May be fixed by #398
Closed

Consider using an upstream icon package #370

xM8WVqaG opened this issue Jul 31, 2019 · 22 comments · May be fixed by #398
Assignees
Projects
Milestone

Comments

@xM8WVqaG
Copy link

Idea reposted from Telegram. Please feel free to edit this issue's title/description.

Preparing and optimising icons doesn't really have much to do with TOTP. If possible, it might be a good idea to investigate outsourcing the icon maintenance to a third party.

There are several interesting icon projects around, one nice example is SuperTinyIcons: https://github.com/edent/SuperTinyIcons

This solves two problems:

  • You can just close tickets about icons and redirect people upstream. Resolves maintenance issues like existing icon consistency: Sizing Icons Proportionally To Each Other #365
  • If the icon package is kept updated and new versions are pulled into andOTP regularly for releases then people will continue to get new icons available in the core without having to use custom icons.
@flocke
Copy link
Member

flocke commented Jul 31, 2019

I really like the idea of outsourcing the maintenance of the icons. And since SuperTinyIcons actually provides a lot of Android vector drawables it should be fairly straight forward to merge them. Maybe I could write a small script that takes care of that.
I will definitely look into it, but don't expect anything to soon, I don't have any free time currently.

@pred2k
Copy link

pred2k commented Sep 12, 2019

I think the service-icons are pretty essential when using this AndOTP.
I have many entries in AndOTP and want to find the right one quickly when loggin in into a service.
Known Icons catch you searching eyes.
When no known icon is present, i have the servicename and my acountname in the description. Which is long, scrolling text and that takes addition time to login.

I like the idea of getting new service icons into AndOTP fast. Maybe SuperTinyIcons can help.
But SuperTinyIcons is still missing ProtonMail, IFTT,...

@vangyyy
Copy link

vangyyy commented Oct 7, 2019

Is this being worked on ? I would like to take a look at this.

@alexanderadam
Copy link

alexanderadam commented Oct 9, 2019

@vangyyy I'm not a maintainer but I have the feeling that @flocke would be very happy if you would provide a PR for this.

EDIT:

@pred2k wrote:
But SuperTinyIcons is still missing ProtonMail

It will probably be fixed soonish. 😉

@flocke
Copy link
Member

flocke commented Oct 9, 2019

@vangyyy Sorry it took me so long to respond. Just as @alexanderadam said, I would be happy if you could look into it. I don't have a lot of time currently.

@edent
Copy link

edent commented Oct 13, 2019

If there are any other icons you would like, please let us know in https://github.com/edent/SuperTinyIcons/issues

@RichyHBM
Copy link
Contributor

@vangyyy Did you get anywhere with this? Might try and get this done today

@vangyyy
Copy link

vangyyy commented Oct 13, 2019

@RichyHBM I am working on it. Though it isn't as straight forward as importing the icons from SuperTinyIcons repository. Couple icons do not work under Android as they are in android-vector-drawable folder. I guess whoever converted them never tested them. I will probably submit some PRs to their repository to get it working as well as to improve coverage (a lot of current andOTP icons are simply not available). As far as implementation goes I was thinking of adding the repository as git submodule, which could be updated between releases. What do you think ?

@RichyHBM
Copy link
Contributor

Yes, I started working on something for this this morning, I have added STI as a submodule and am adding the icons via a gradle task. I imagine most of the work for this will be done in gradle as to automate as much as possible

@edent
Copy link

edent commented Oct 13, 2019

@vangyyy I'm the maintainer of SuperTinyIcons. Pull requests very welcome. The android-vectors were automatically converted, not sure why they don't work - but guidance welcome!

@RichyHBM
Copy link
Contributor

Hey @edent I opened an issue on SuperTinyIcons about the broken vector images, I also noticed a could of them may be the wrong size? Hopefuly that will be fixed with SuperTinyIcons#189

Anyway, I have got it all working with andOTP in an automated way, we would just need to decide wether to exclusively use STI or how we would do it @flocke. For now I have removed our versions that were also in STI

@vangyyy
Copy link

vangyyy commented Oct 13, 2019

I have submitted a PR in STI that fixes the issue and also adds existing icons that were missing in android-vector-drawable folder. As for the missing icons from andOTP, I would suggest leaving them for now, but maybe create an issue in STI where we could list them and work on them.

@flocke
Copy link
Member

flocke commented Oct 13, 2019

Thank you @vangyyy and @RichyHBM for working on this. The PR looks really good, but I haven't had time to test it yet. And also thanks a lot @edent for SuperTinyIcons, I didn't know of it before this PR but it looks really cool.

@vangyyy
Copy link

vangyyy commented Oct 18, 2019

Upstream PR that fixes android drawables was merged. All of them should work now. There is one problem though. STI doesn't require from contributors generating android drawables, they are optional. So in the future newly added icons may become broken or worse ... may not be generated at all.

@alexanderadam
Copy link

STI doesn't require from contributors generating android drawables

Maybe @edent could consider enforcing Android Vector Drawables for new icons the in the Guidelines & PRs.

What do you think @edent?
This way the availability of the icons would be consistent.

@vangyyy
Copy link

vangyyy commented Oct 18, 2019

The problem with this is that it will become a burden for contributors and may result in less icons being made for STI. The conversion itself can not be easily automated with a script. I found a tool that produces correct image 99% of the time. It is payed though. It can only be used once (more if you open new incognito session every time ... cookies). Another solution is to convert them manually before release, but that defeats the whole point of using it in a first place.

@alexanderadam
Copy link

alexanderadam commented Oct 18, 2019

The conversion itself can not be easily automated with a script

Why not?
Shouldn't it be possible with the command line interface of Android Studio?
I just searched and found this blog entry that says that it should be possible with

$ ./vd-tool -c -in svgs/ -out vectors/

So couldn't this step added to the travis configuration where the build seem to happen anyway?

PS: I'm not an Android or Java developer so please excuse if I am missing fundamental things here.

@RichyHBM
Copy link
Contributor

I don't really see an issue, if an icon is missing a drawable it can just be done as an issue/pr, same as if the icon didn't exist

@NeverDucky
Copy link

There is also the option of collaborating with https://github.com/2factorauth/twofactorauth for this. The project already maintains a list of icons specifically tailored to two-factor-authentication-providing services. Both projects are related and would do well to share resources.

@alexanderadam
Copy link

The project already maintains a list of icons specifically tailored to two-factor-authentication-providing services.

Are they also offering vector icons? So far I just found low resolution raster images (i.e. those) which obviously will look ugly on high resolution displays.

@NeverDucky
Copy link

NeverDucky commented Oct 25, 2019

Are they also offering vector icons? So far I just found low resolution raster images (i.e. those) which obviously will look ugly on high resolution displays.

As far as I can tell, no. That would be the downside of using those icons.

@flocke flocke modified the milestones: v0.8.0, v0.8.1 Jul 7, 2020
@flocke flocke added this to Todo in Thumbnails via automation Jan 30, 2021
@flocke flocke moved this from Requests to Issues in Thumbnails Jan 30, 2021
@flocke
Copy link
Member

flocke commented Mar 3, 2021

Closing this issue as we have a PR and all further discussion should happen there: #398

@flocke flocke closed this as completed Mar 3, 2021
Thumbnails automation moved this from Issues to Done Mar 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging a pull request may close this issue.

8 participants