D17751: Add shadow to Hour's hand

This revision was automatically updated to reflect the committed changes. Closed by commit R120:975df8f8930c: Add shadow to Hour's hand (authored by shubham). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17751?vs=49728&id=49796 REVISION DETAIL ht

D17751: Add shadow to Hour's hand

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Land on master only please, as this requires a change in Frameworks 5.55. REPOSITORY R120 Plasma Workspace BRANCH hand REVISION DETAIL https://phabricator.kde.org/D17751 To: shubh

D17751: Add shadow to Hour's hand

shubham updated this revision to Diff 49728. shubham added a comment. Updating D17751 <https://phabricator.kde.org/D17751>: Add shadow to Hour's hand Use harcoded pixel values REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17751?

D17751: Add shadow to Hour's hand

ngraham added a comment. > `units.smallSpacing * [some value]` I'm afraid this won't work, since it will result in fractional pixel values and blurry edges. For this, you need to just hardcode the number of pixels. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator

D17751: Add shadow to Hour's hand

shubham updated this revision to Diff 49656. shubham added a comment. 1. Set the hour hand shadow's topMargin to -6px and second hand shadow's topMargin to 2px 2. Use units.smallSpacing instead of hardcoding the values REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://

D17751: Add shadow to Hour's hand

ngraham added a comment. D18288 has landed! REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D17751 To: shubham, mart, hein, #vdg, ngraham, davidedmundson Cc: ndavis, abetts, ngraham, davidedmundson, plasma-devel, jraleigh,

D17751: Add shadow to Hour's hand

shubham added a comment. ngraham I will put those changes in effect REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D17751 To: shubham, mart, hein, #vdg, ngraham Cc: ndavis, abetts, ngraham, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenB

D17751: Add shadow to Hour's hand

ngraham added a comment. The SVG did actually need some tweaking. I've done that at D18288 . After that, I found that setting the hour hand shadow's `topMargin` to -6 and changing the second hand's `topMargin` to 2 looked great: F6551260: Screensh

D17751: Add shadow to Hour's hand

mart added a comment. In D17751#381215 , @davidedmundson wrote: > From the linked bug report (and your screenshot) > > > You're right about the typo, but it doesn't seem that trivial. The shadow shadow appears all offset (on the default bree

D17751: Add shadow to Hour's hand

ndavis added a comment. In D17751#382113 , @shubham wrote: > In D17751#381435 , @ndavis wrote: > > > No, the SVG that the analog clock uses is in plasma-framework > > > Can you tell me how to fi

D17751: Add shadow to Hour's hand

shubham added a comment. In D17751#381435 , @ndavis wrote: > In D17751#381431 , @shubham wrote: > > > How could the shadow appearance be fixed? In breeze icons? > > > No, the SVG that the analo

D17751: Add shadow to Hour's hand

ndavis added a comment. In D17751#381431 , @shubham wrote: > How could the shadow appearance be fixed? In breeze icons? No, the SVG that the analog clock uses is in plasma-framework REPOSITORY R120 Plasma Workspace REVISION DETAIL ht

D17751: Add shadow to Hour's hand

shubham added a comment. How could the shadow appearance be fixed? In breeze icons? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D17751 To: shubham, mart, hein, #vdg, ngraham Cc: abetts, ngraham, davidedmundson, plasma-devel, GB_2, ragreen, Pitel, ZrenBot

D17751: Add shadow to Hour's hand

abetts added a comment. In D17751#381351 , @ngraham wrote: > Gotta fix the shadow appearance before we can land this. > > If we consider where the light is actually coming from, probably we need for the center part to cast a shadow too.

D17751: Add shadow to Hour's hand

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. Gotta fix the shadow appearance before we can land this. If we consider where the light is actually coming from, probably we need for the center part to cast a shadow too.

D17751: Add shadow to Hour's hand

davidedmundson added a comment. From the linked bug report (and your screenshot) > You're right about the typo, but it doesn't seem that trivial. The shadow shadow appears all offset (on the default breeze theme at least) The hour hand shadow starts several px below the hour hand RE

D17751: Add shadow to Hour's hand

shubham created this revision. shubham added reviewers: mart, hein. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. shubham requested review of this revision. TEST PLAN Add new analog clock widget from "Add widget" on right click context m REPOSITORY R120 Plasma Works