Differential D24853

[DigitalClock] Fix layout and QML warnings
ClosedPublic
Actions

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

Details

Reviewers
ngraham
Group Reviewers
Plasma
Plasma: Workspaces
Commits
R120:82cfabd0a5c8: [DigitalClock] Fix layout and QML warnings
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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
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
davidedmundson added a subscriber: davidedmundson.Oct 22 2019, 9:54 AM

Concept +1, I love layouts

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

Do you need these?

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

54

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

88

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
31–32

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

54

Cool! I need there must be something!

88

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

ngraham added a subscriber: ngraham.Oct 22 2019, 1:55 PM

Some of this partially conflicts with D24798 FWIW.

kmaterka marked 3 inline comments as done.Oct 22 2019, 1:58 PM
In D24853#552228, @ngraham wrote:

Some of this partially conflicts with D24798 FWIW.

I can wait and rebase later - your decision.

kmaterka added a comment.Oct 24 2019, 9:42 AM

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.

ngraham added a comment.Oct 24 2019, 3:27 PM

Fair enough, that seems reasonable.

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

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
58

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
Closed by commit R120:82cfabd0a5c8: [DigitalClock] Fix layout and QML warnings (authored by kmaterka). · Explain WhyOct 25 2019, 1:36 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 68745

View Options

applets/digital-clock/package/contents/ui/configAppearance.qml

Loading...
View Options

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

Loading...