[DigitalClock] Fix layout and QML warnings
ClosedPublic

Authored by kmaterka on Oct 22 2019, 9:29 AM.

Details

Summary

Fixed issue when time zone configuration page was not scaling corrently. As a result two
srollbars were rendered.
Fixed few QML layout warnings and undefined references.

Test Plan

Open Time zones configuration page and shrink window - two scrollbars are rendered.
Expected: table is scalled correctly and only one scrollbar in table is rendered.

Diff Detail

Repository
R120 Plasma Workspace
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18019
Build 18037: arc lint + arc unit
kmaterka created this revision.Oct 22 2019, 9:29 AM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 22 2019, 9:29 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
kmaterka requested review of this revision.Oct 22 2019, 9:29 AM

Concept +1, I love layouts

applets/digital-clock/package/contents/ui/configTimeZones.qml
32

Do you need these?

Generally it's a sign of a problem or bad layering if the root item has anchors in it.

56

If we're going to port stuff, we may as well port to:

https://api.kde.org/frameworks/kirigami/html/classorg_1_1kde_1_1kirigami_1_1InlineMessage.html

the library version of this

96

What's this for? Opacity should be inherited by children

kmaterka marked 3 inline comments as done.Oct 22 2019, 10:24 AM
kmaterka added inline comments.
applets/digital-clock/package/contents/ui/configTimeZones.qml
32

Ops, copy&pPaste from HolidayConfig.qml. I'll remove it, width/height are not needed, I just checked, AppletConfiguration.qml handles that.

56

Cool! I need there must be something!

96

I will use Kirigami component.

kmaterka updated this revision to Diff 68524.Oct 22 2019, 10:24 AM
kmaterka marked 3 inline comments as done.

Review changes: Kirigami component, removed anchors

Some of this partially conflicts with D24798 FWIW.

kmaterka marked 3 inline comments as done.Oct 22 2019, 1:58 PM

Some of this partially conflicts with D24798 FWIW.

I can wait and rebase later - your decision.

It looks like there are some review changes required in D24798. Maybe I can commit my changes first? D24798 needs changes anyway and it would be good to split QCC1 -> QCC2 migration from layout fix.

Fair enough, that seems reasonable.

applets/digital-clock/package/contents/ui/configTimeZones.qml
60

Don't use hardcoded margins. This should be Kirigami.Units.smallSpacing

kmaterka added inline comments.Oct 25 2019, 1:03 PM
applets/digital-clock/package/contents/ui/configTimeZones.qml
60

I just copied values form the original Rectangle. I will remove it entirely, probably it's not needed anymore.

kmaterka updated this revision to Diff 68743.Oct 25 2019, 1:08 PM

Remove hardcoded margins, these are not needed anymore.

kmaterka marked 2 inline comments as done.Oct 25 2019, 1:08 PM
ngraham accepted this revision.Oct 25 2019, 1:20 PM

LGTM.

This revision is now accepted and ready to land.Oct 25 2019, 1:20 PM
This revision was automatically updated to reflect the committed changes.