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

ICO/ICNS Revision #489

Merged
merged 2 commits into from
Sep 26, 2018
Merged

ICO/ICNS Revision #489

merged 2 commits into from
Sep 26, 2018

Conversation

rossmoody
Copy link
Contributor

Fixes brave/brave-browser#1217 and possibly fixes brave/brave-browser#1237. Regardless these are non damaging positive aesthetic updates for the app icons to render edge case sizing and older operating system icons better.

  • stylistically nothing in these icons have changed. they have the same appearance and logic as Channel Icon Updates #437
  • structurally the icons now each contain additional system sizes to account for correct rendering in previous operating systems where 1024x1024 wasn't being scaled (specifically sub 10.13 releases)
  • this includes 16x16, 32x32, 48x48 and 256x256 for Windows ico files
  • this includes 16x16, 32x32, 128x128, 256x256 and 512x512 in both regular and @2x retina resolutions for ICNS app files
  • i noticed a document .icns file hanging out in the theme folder with a chromium logo. unsure where this lives user facing but regardless all chromium logos must be banished

- stylistically nothing in these icons have changed. they have the same appearance and logic as PR#437
- structurally the icons now each contain additional system sizes to account for correct rendering in previous operating systems where 1024x1024 wasn't being scaled (specifically sub 10.13 releases)
- this includes 16x16, 32x32, 48x48 and 256x256 for Windows ico files
- this includes 16x16, 32x32, 128x128, 256x256 and 512x512 in both regular and @2x retina resolutions for ICNS app files
- i noticed a document .icns file hanging out in the theme folder with a chromium logo. unsure where this lives user facing but regardless all chromium logos must be banished
simonhong
simonhong previously approved these changes Sep 22, 2018
Copy link
Member

@simonhong simonhong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

- updated a grammatical error for 'The Senate"
- noticed two small grammatical errors related to colons
@rossmoody
Copy link
Contributor Author

Made an additional small commit attached to this for revising a few grammatical errors in the NTP. Fixes brave/brave-browser#1271

@bbondy bbondy merged commit 3bab659 into master Sep 26, 2018
bbondy added a commit that referenced this pull request Sep 26, 2018
@bbondy
Copy link
Member

bbondy commented Sep 26, 2018

master: 3bab659
0.55.x 262b0fd

@bsclifton bsclifton deleted the icns-ico-updates branch September 26, 2018 05:43
@bbondy bbondy added this to the 0.55.x - Release milestone Jan 14, 2019
petemill pushed a commit to petemill/brave-core that referenced this pull request Jul 5, 2019
…ly-tip-usd

don't show USD conversion in wallet monthly tip
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants