| ahiemstra | |
| broulik | |
| mart |
| KNewStuff | |
| Frameworks | |
| Plasma | |
| VDG |
This switches from using the QML DropShadow to using the
new, high efficiency ShadowedRectangle in Kirigami.
No visible changes (as expected) from the switch
Minor visible change from the EntryScreenshots modifications (now correctly shows the shadows all the way around)
Screenshots component without changes:
Screenshots component with changes:
| No Linters Available |
| No Unit Test Coverage |
| Buildable 24136 | |
| Build 24154: arc lint + arc unit |
| src/qtquick/qml/private/EntryScreenshots.qml | ||
|---|---|---|
| 133 | Not needed given you anchors.fill | |
| 135 | What's this for? | |
| 138 | You used largeSpacing as verticalOffset before | |
| 139 | You used 12 as radius before | |
| src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml | ||
| 123 | shadow.size corresponds to the old radius, so you'd want to set Kirigami.Units.largeSpacing here | |
| 124 | #80 used in the old code is 128, i.e. 0.5 alpha, unless you want to make it look consistently with the shadows used elsewhere? | |
| src/qtquick/qml/private/EntryScreenshots.qml | ||
|---|---|---|
| 133 | Ah yes, quite :) | |
| 135 | That's a good question, really - i found that in the KCM GridDelegate and just sort of left it in... but yup, probably not needed :) | |
| src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml | ||
| 123 | Less magic numbers are a nice thing ;) | |
| 124 | Yeah, that was for consistency, but well spotted :) | |
(pending some screenshots of the visible change)
| src/qtquick/qml/private/EntryScreenshots.qml | ||
|---|---|---|
| 138 | Hm, on second thought, it looks /terrible/ doing it that way... i'm going to call consistency on this one and tag in the VDG, thanks for making me notice :) | |
Just an observation, we should maybe consider making a shared component for this screenshots list so that Discover can use it too.
i would like that :) Perhaps something for kirigami-addons? (was that @davidedmundson or someone else?)
| src/qtquick/qml/private/EntryScreenshots.qml | ||
|---|---|---|
| 134 | The x and y offsets should be 0 by default, so there's little reason to specify that I'd say. | |
| src/qtquick/qml/private/EntryScreenshots.qml | ||
|---|---|---|
| 134 | Ha, yes, quite, setting defaults does seem a little silly ;) | |
Ping? Not super critical, i guess, but it just feels sad to have things pending for weeks...
It's fine with me but I'm going to leave the final call to @broulik since he did a more in-depth review.