Skip to content
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

upstream from suru-update-201703 #4

Merged
merged 4 commits into from Apr 24, 2019

Conversation

@mymike00
Copy link
Contributor

commented Mar 25, 2019

I was unsure how to merge debian/changelog and it was a bit messy with mic icons @cibersheep

@cibersheep

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2019

Sorry for the mic-gate :D I'll to add those after this update.
Do we need the png? I don't think we are using them in the system...

Show resolved Hide resolved debian/changelog
@mymike00

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

Do we need the png? I don't think we are using them in the system...

will they ever be useful? if not I agree with you, but if there's a chance they'll be useful in the future and they don't waste too much space then we can keep them

@cibersheep

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

I would suggest to add the png if we find we use them. I think we should use svg when possible (we already do, I think)

@NeoTheThird

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Where performance matters, pngs can be more efficiently rendered on the gpu than svgs.

@cibersheep

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

Qt renders svg to the size you tell it. If you don't set the size of the svg relative to the parent width should be ok.
Space Invader was made exclusively by svg and you couldn't feel any difference :)

@mymike00

This comment has been minimized.

Copy link
Contributor Author

commented Apr 5, 2019

well, let me know which is best and what to do, I'd like to merge this asap!

@cibersheep

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2019

I would just push the svg, if we miss any png we'll solve that later (as we don't use them now).

mymike00 added some commits Apr 5, 2019

Show resolved Hide resolved debian/changelog Outdated
@UniversalSuperBox

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

@NeoTheThird brought up a good point, this could potentially break apps which are relying on, say, the Language and Text icon being a globe. Might need to wait for OTA-10's cycle.

@mymike00

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

this could potentially break apps which are relying on, say, the Language and Text icon being a globe.

sorry, I don't understand what you're referring to

Might need to wait for OTA-10's cycle.

I hope we can find a solution, I'd like to see this merged asap...

@UniversalSuperBox

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

Wait, I think I misunderstood. Looks like icons were not normally replaced, just new ones added. Good. I'll do a final check through them.

@UniversalSuperBox
Copy link
Member

left a comment

The only icon that was replaced rather than tweaked is preferences-desktop-locale-symbolic. The icon is also available as language-chooser, which it was symlinked to before.

I think the risk that an application embedded this icon specifically as preferences-desktop-locale-symbolic is rather low and can be changed easily.

@UniversalSuperBox UniversalSuperBox merged commit 6d37ad6 into ubports:xenial Apr 24, 2019

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.