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 Timestripe icons #3592

Merged
merged 5 commits into from
Nov 8, 2023
Merged

Conversation

yevklim
Copy link
Contributor

@yevklim yevklim commented Nov 6, 2023

Timestripe is a time-management web application with the official website at https://timestripe.com.

On a desktop, it can be installed as a PWA in most Chromium-based browsers.

With this PR, I'm adding icons for Timestripe and icons-symlinks for the PWA for Brave, Chrome, and Vivaldi.

The icons of sizes 16x16, 22x22, and 24x24 look different from the higher-resolution icons because that's what the official icons of that size look like.

@yevklim yevklim marked this pull request as draft November 7, 2023 09:36
@yevklim yevklim marked this pull request as ready for review November 7, 2023 10:20
@SmartFinn
Copy link
Member

Ha, it's the strangest way to add the light outline that I have seen. You may keep it, but it less maintainable. Also, #4f4f4f is the darker base color allowed in this icon theme, please change it.

@yevklim
Copy link
Contributor Author

yevklim commented Nov 7, 2023

Initially, I made the outline a path like on most other icons in Papirus. However, using a simple rounded rectangle takes fewer characters in SVG, and it also seemed more maintainable as it's easier to change the radius, for example.

As for the color, I actually did my own research, looking for the darkest base color among Papirus icons. Most icons do have the darkest color at 31% lightness (#4f4f4f), as you defined, but I also found a few icons with a darker base color of 25% lightness (#3f3f3f). And so, I chose 25% lightness (#404040). Anyway, I respect the rules and will change the base color to 31% lightness for my icon.

Other icons with the base color at 25% lightness are angrysearch, apollo-studio, plex, tidal, pacseek, and a dozen of others too. Am I misunderstanding what the base color is here?

@yevklim
Copy link
Contributor Author

yevklim commented Nov 7, 2023

Updated the base color to #4f4f4f

@SmartFinn
Copy link
Member

@yevklim

However, using a simple rounded rectangle takes fewer characters in SVG, and it also seemed more maintainable as it's easier to change the radius, for example

I can't easily remove the shadows and the light outlines with sed by pattern fill:#ffffff;opacity:0.2, in this case.

Other icons with the base color at 25% lightness are angrysearch, apollo-studio, plex, tidal, pacseek, and a dozen of others too. Am I misunderstanding what the base color is here?

These may be exceptions, or these icons were added before HIG.

@yevklim
Copy link
Contributor Author

yevklim commented Nov 7, 2023

Thanks for the guidelines. I will follow them more closely in this and my future contributions.

I will change the shadow and light outline as you suggested.

@SmartFinn
Copy link
Member

SmartFinn commented Nov 7, 2023

@yevklim the shadows for inner object are missing too. Could you added it with unset fill and opacity:0.1:

image

@yevklim yevklim marked this pull request as draft November 7, 2023 13:06
@yevklim yevklim marked this pull request as ready for review November 8, 2023 12:00
@yevklim
Copy link
Contributor Author

yevklim commented Nov 8, 2023

@SmartFinn

I made changes following the guidelines and your comments. Hope I didn't miss anything. Please review.

- Reduce opacity of shadows for inner objects
- Replace bright white color with #e0e0e0e
- Add missing `ry=` attributes for rounded rectangles
- Replace rounded square with squircle for 16-24px icons to unify the shape
- Replace shadow path object with rect
- Fix blurred edge for inner object in 48px icon
@SmartFinn
Copy link
Member

SmartFinn commented Nov 8, 2023

@yevklim Thanks. This is a very good icon for the first time.

@SmartFinn SmartFinn merged commit c0a9dd0 into PapirusDevelopmentTeam:master Nov 8, 2023
@yevklim yevklim deleted the timestripe branch January 4, 2025 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants