Adjust date string height to match time string with vertical panel
ClosedPublic

Authored by antnyzilla on Feb 21 2019, 7:22 AM.

Details

Summary

Changes the height of the date string to match that of the
time string so the date is readable with a vertical panel

BUG: 404611

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
antnyzilla created this revision.Feb 21 2019, 7:22 AM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 21 2019, 7:22 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
antnyzilla requested review of this revision.Feb 21 2019, 7:22 AM

Before

After

antnyzilla retitled this revision from # Enter a commit message. # # Changes: # # applets/digital-clock/package/contents/ui/DigitalClock.qml Adjust date string height to match time string with vertical panel to Adjust date string height to match time string with vertical panel.Feb 21 2019, 8:01 AM
anthonyfieroni added inline comments.
applets/digital-clock/package/contents/ui/DigitalClock.qml
305

You should check if panel orientation, now it's broken horizontal view?

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.

abetts added a subscriber: abetts.Feb 21 2019, 2:04 PM

I would probably add just a bit of left and right padding to the time as well. LGTM

ngraham added subscribers: Zren, davidedmundson, ngraham.

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 visual regressions when using a horizontal or vertical panel, either with the default height, or a taller/wider height. The code change looks sane on the surface (more than sane even, since it's removing the use of a magic number), however I'm not super familiar with this code so I'd like @Zren and/or @davidedmundson to sign off on this first.

Zren added inline comments.Feb 22 2019, 6:12 PM
applets/digital-clock/package/contents/ui/DigitalClock.qml
305

This is inside a State { when: plasmoid.formFactor == PlasmaCore.Types.Vertical }

mart added a subscriber: mart.Feb 26 2019, 10:42 AM
cfeck added a subscriber: cfeck.Mar 12 2019, 9:35 PM
cfeck added inline comments.
applets/digital-clock/package/contents/ui/DigitalClock.qml
305

Could you please clarify? Are changes needed?

ngraham accepted this revision.Mar 13 2019, 3:06 PM

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.

This revision is now accepted and ready to land.Mar 13 2019, 3:06 PM
rooty accepted this revision.Mar 13 2019, 3:45 PM
rooty added a subscriber: rooty.

+1 from me too

ngraham added inline comments.Mar 13 2019, 3:52 PM
applets/digital-clock/package/contents/ui/DigitalClock.qml
305

No changes are needed; that comment was indicating why @anthonyfieroni's comment was not applicable.

This revision was automatically updated to reflect the committed changes.