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

Update dashboard icon #46271

Merged
merged 1 commit into from
Jul 7, 2024
Merged

Update dashboard icon #46271

merged 1 commit into from
Jul 7, 2024

Conversation

AndyScherzinger
Copy link
Member

  • Resolves: none

Summary

replace icon with the exact sizes of other icons, like activity, so the original viewport size matches height/width, to be rendered more crisp

Checklist

@AndyScherzinger AndyScherzinger added this to the Nextcloud 30 milestone Jul 3, 2024
@AndyScherzinger AndyScherzinger added design Design, UI, UX, etc. 3. to review Waiting for reviews labels Jul 7, 2024
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@AndyScherzinger AndyScherzinger merged commit f6b53bb into master Jul 7, 2024
110 checks passed
@AndyScherzinger AndyScherzinger deleted the enh/dashboard-icon branch July 7, 2024 17:59
@susnux
Copy link
Contributor

susnux commented Jul 7, 2024

@AndyScherzinger where did you take the icon from? It seems to be not the Material Symbol with grade: 0, optical size: 20 weight: 400. For consistent style we need to use the 20px optical size version everywhere.

But funny seems like we have a bug here (unrelated to this changes). I noticed that with this version the shown icon is 32px in size, while we normally want to enforce 20px always, but the styles are broken...

expected now
Bildschirmfoto_20240707_211501 Bildschirmfoto_20240707_211333

@susnux
Copy link
Contributor

susnux commented Jul 7, 2024

Moreover this again breaks theming as you have removed the fill="#fff" No this one is an unrelated bug 🙈

@AndyScherzinger
Copy link
Member Author

@susnux I took the icon from Material Symbols. Can't say for sure which size though.
I can just do another PR on Monday with the optical size 20 yet when I checked other app icons their svg size was 32,no matter the optical size.
Can't say if that makes a difference here.

Also with the other apps I might even say the files app icon is the one smaller than the others?

@susnux
Copy link
Contributor

susnux commented Jul 7, 2024

Also with the other apps I might even say the files app icon is the one smaller than the others?

Thats a bug. In the menu all icons should be 20px but a styling update broke the selector so that now the original size is shown.

@AndyScherzinger
Copy link
Member Author

Ah, okay. Now I got it. Thanks for explaining it again. So what is the way forward? Style gets fixed or also the icon needs to be changed in size within the svg?

@susnux
Copy link
Contributor

susnux commented Jul 8, 2024

Well I would say if you selected this style of the icon for all other apps now, then lets stick with it.
I will see if I find some time fixing styles, but lets keep it consistent :)

@AndyScherzinger
Copy link
Member Author

Well, the actual size might have been slightly to big. So I adjusted it to match the others, i.e. the photos app icon #46360

@blizzz blizzz mentioned this pull request Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews design Design, UI, UX, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants