rkflx added a comment.
New year, new look ;) Great work, this looks impressive and solves the left-side usability problem at the same time. The screenshots above show a different shadow size for windows and menus respectively. This is actually something I like, because having a smaller shadow means having the illusion of a smaller z-height between menu and window, so you get a better feel that the menu belongs to the current window. Note this means in case a menu extends beyond the window we'd now have two different shadow sizes above another single window in some cases, i.e. like this: F5611051: shadow-sizes.png <https://phabricator.kde.org/F5611051> I welcome this change and assume this is a conscious decision of the https://phabricator.kde.org/tag/vdg/ for desktop style GUIs, because IMO usability and looks are more important than perfectly replicating the physics of light. Shadows of Plasma widgets/popups are already centered and smaller anyway, and it alleviates the problem mentioned by Christoph (nested menus). So +1 from me. --- However, the single spin box in the shadow config dialog of Breeze still affects both at the same time, meaning if you decrease the default from 64px to 63px, the menu shadow will suddenly get very large. This could be solved in two ways: - Duplicate / split the config options for menus/tooltips/… and windows. - Set a static size for the menu/tooltip/… shadow, only make the window shadow size configurable. (← I'd prefer this.) Also, I wonder whether the maximum value of the spin box should be increased. In general a default value means the user can increase or decrease something to his liking, while now he can only decrease this. Regarding closing the other Diff: If we don't center the Breeze widget style at the same time (which personally I don't think is mandatory), we could still provide the option do disable centering of shadows. As can be read in the comments, some like off-centered shadows which is a unique offering of the Plasma desktop. After all, the code needed for this should not be too complex. Maybe @rpelorosso could work on this and on the RTL issue in https://phabricator.kde.org/D8232? Last thing I want to mention is HiDPI: IMO in addition to "must work on Wayland" for any new feature / default, we should have the policy of "must work on HiDPI" (could be a dependent patch, though). Currently, the bigger shadow size only scales correctly for menu shadows, but not for window shadows (top: 2x scaling, bottom: 1x scaling doubled in size retroactively): F5611636: hidpi-shadows.png <https://phabricator.kde.org/F5611636> --- TL;DR, I'd like to see: - menu shadow size not affected by config dialog - increased maximum value - option to disable centering, RTL aware - fix HiDPI window shadow size REPOSITORY R31 Breeze BRANCH master REVISION DETAIL https://phabricator.kde.org/D9549 To: ngraham, abetts, hpereiradacosta, #vdg, #breeze, alake Cc: rkflx, zzag, cfeck, januz, rpelorosso, apol, mvourlakos, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, mart
