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

Add some background protection to Appcenter app listings #118

Merged
merged 1 commit into from Apr 8, 2019

Conversation

@isantop
Copy link

@isantop isantop commented Feb 28, 2019

New GNOME app icons don't always include a built-in stroke for background
protection. This means that they can sometimes become lost against light-colored
backgrounds and are difficult to see.

This adds a slight icon shadow is certain contexts where such an icon may
become hard to see, such as search results or app listings.

Before this change:
screenshot from 2019-02-28 10-25-08

After:
screenshot from 2019-02-28 10-25-16

New GNOME app icons don't always include a built-in stroke for background
protection. This means that they can sometimes become lost against light-colored
backgrounds and are difficult to see.

This adds a slight icon shadow is certain contexts where such an icon may
become hard to see, such as search results or app listings.
@isantop isantop self-assigned this Feb 28, 2019
@cassidyjames
Copy link

@cassidyjames cassidyjames commented Feb 28, 2019

Note that while the new GNOME icon style is supposed to include icons rendered by the OS, most app icons provided by third parties and for KDE, XFCE, elementary, etc. will include shadows rendered as part of the icon. So this might end up with extra heavy, double shadows

@isantop
Copy link
Author

@isantop isantop commented Feb 28, 2019

@brs17 Design feedback would be appreciated before I get this ready to review.

@isantop
Copy link
Author

@isantop isantop commented Feb 28, 2019

@cassidyjames The shadows I have right now seem to blend fairly well with most of the baked-in shadows I've seen. Here's a few examples:

screenshot from 2019-02-28 13-46-34

screenshot from 2019-02-28 13-47-51

We also aren't drawing them when we're viewing the app overview page, which definitely helps keep things lightweight and preserve the application identity:
screenshot from 2019-02-28 13-48-49

@isantop
Copy link
Author

@isantop isantop commented Mar 5, 2019

In the absence of further comments, I'm going to mark this ready for review.

@isantop isantop marked this pull request as ready for review Mar 5, 2019
@isantop isantop requested review from brs17 and jackpot51 Mar 5, 2019
mmstick
mmstick approved these changes Mar 5, 2019
Copy link

@mmstick mmstick left a comment

Everything looks good to me.

@isantop isantop removed the request for review from jackpot51 Mar 6, 2019
brs17
brs17 approved these changes Apr 8, 2019
@isantop
Copy link
Author

@isantop isantop commented Apr 8, 2019

@mmstick I apparently can't merge this.

@brs17 brs17 merged commit 3a86893 into master Apr 8, 2019
6 checks passed
@brs17 brs17 deleted the feature-app-icon-background-protection branch Apr 8, 2019
@brs17
Copy link
Member

@brs17 brs17 commented Apr 8, 2019

Merged! 🔥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants