D19194: Adjust date string height to match time string with vertical panel

This revision was automatically updated to reflect the committed changes. Closed by commit R120:8f1b10b23a96: Adjust date string height to match time string with vertical panel (authored by antnyzilla, committed by ngraham). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE

D19194: Adjust date string height to match time string with vertical panel

ngraham added inline comments. INLINE COMMENTS > cfeck wrote in DigitalClock.qml:305 > Could you please clarify? Are changes needed? No changes are needed; that comment was indicating why @anthonyfieroni's comment was not applicable. REPOSITORY R120 Plasma Workspace BRANCH newclock2

D19194: Adjust date string height to match time string with vertical panel

rooty accepted this revision. rooty added a comment. +1 from me too REPOSITORY R120 Plasma Workspace BRANCH newclock2 (branched from master) REVISION DETAIL https://phabricator.kde.org/D19194 To: antnyzilla, #vdg, #plasma, davidedmundson, Zren, ngraham, rooty Cc: rooty, cfeck, mart,

D19194: Adjust date string height to match time string with vertical panel

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. No, changes are not needed. This works great for me and the code makes sense. I could not find any regressions in my testing. I recommend landing it. REPOSITORY R120 Plasma

D19194: Adjust date string height to match time string with vertical panel

cfeck added inline comments. INLINE COMMENTS > Zren wrote in DigitalClock.qml:305 > This is inside a `State { when: plasmoid.formFactor == > PlasmaCore.Types.Vertical }` Could you please clarify? Are changes needed? REPOSITORY R120 Plasma Workspace REVISION DETAIL

D19194: Adjust date string height to match time string with vertical panel

Zren added inline comments. INLINE COMMENTS > anthonyfieroni wrote in DigitalClock.qml:305 > You should check if panel orientation, now it's broken horizontal view? This is inside a `State { when: plasmoid.formFactor == PlasmaCore.Types.Vertical }` REPOSITORY R120 Plasma Workspace REVISION

D19194: Adjust date string height to match time string with vertical panel

ngraham added subscribers: Zren, davidedmundson, ngraham. ngraham added reviewers: Plasma, davidedmundson, Zren. ngraham added a comment. Thank you for the patch! You did great, and on the first try, too! I can confirm that this change fixes the issue for me and I do not notice any

D19194: Adjust date string height to match time string with vertical panel

abetts added a comment. I would probably add just a bit of left and right padding to the time as well. LGTM REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19194 To: antnyzilla, #vdg Cc: abetts, anthonyfieroni, plasma-devel, jraleigh, GB_2, ragreen, Pitel,

D19194: Adjust date string height to match time string with vertical panel

antnyzilla added a comment. Is the panel orientation checked on lines 101, 183, and 256? I haven't noticed any changes or problems with the horizontal panel with this change on my system. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19194 To:

D19194: Adjust date string height to match time string with vertical panel

anthonyfieroni added inline comments. INLINE COMMENTS > DigitalClock.qml:305 > // this can be marginal bigger than contentHeight because of > the horizontal fit > -height: Math.max(0.8 * timeLabel.height, minimumPixelSize) > +height: