-
Notifications
You must be signed in to change notification settings - Fork 745
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
Panel icon size up to 128px #11690
base: master
Are you sure you want to change the base?
Panel icon size up to 128px #11690
Conversation
Just one thing about the icon sizes... most themes only have 32, 48, 64, 96, 128... there are no 40, 56..etc. These are there to make sure the icons are crisp. |
Good point... irrelevant with SVG icons, but with will 56 be downscaled from 64 or upscaled from 48? |
@@ -42,7 +42,7 @@ const APPLETS_DROP_ANIMATION_TIME = 0.2; | |||
const PANEL_PEEK_TIME = 1500; | |||
|
|||
const EDIT_MODE_MIN_BOX_SIZE = 25; | |||
const VALID_ICON_SIZE_VALUES = [-1, 0, 16, 22, 24, 32, 48]; | |||
const VALID_ICON_SIZE_VALUES = [-1, 0, 16, 22, 24, 32, 48, 64, 72, 80, 96, 128]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you missing additional sizes here? (40, 56)
I tried this out, and one issue is that it immediately changes the look of my panels. The default panel size is 40. Currently, that size's optimal color icon is 32px ( The new behavior would now return 40 ( It's important to us to not impact users' existing desktop |
@clefebvre is right. This should not use sizes that are not typically contained in icon themes. As soon as you scale you will get blurry icons. You also cannot depend on themes using SVG as many do not. |
Yeah, it needs some testing and polish, to only downscale when there is higher res. |
I found current max very limiting, especially in multi-monitor configs. There can be all kinds of combinations and needs (personally i use 40 and 64px now). Potentially even 128 might be not quite enough in some setups, as we're pushing above 4K now (yes, i know about UI scaling, it's not always applicable)