Add TabKCM
Needs ReviewPublic

Authored by nicolasfella on Tue, Sep 10, 1:42 PM.

Details

Reviewers
mart
Summary

We have quite a few KCMs that use tabs as top-level components. Since tabs are a bit weird in QML quite a few workarounds have been piling up in some KCMs, making them inconsistent with each other.

By adding a common TabKCM API we can unify the presentation and implementation

Test Plan

Patched plasma-pa KCM:

Diff Detail

Repository
R296 KDeclarative
Branch
tabkcm
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16328
Build 16346: arc lint + arc unit
nicolasfella created this revision.Tue, Sep 10, 1:42 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptTue, Sep 10, 1:42 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
nicolasfella requested review of this revision.Tue, Sep 10, 1:42 PM
nicolasfella edited the test plan for this revision. (Show Details)Tue, Sep 10, 1:43 PM

Nice, I think we need this.

From the image, I think the frame's background color needs to be the same as the active tab color, and there shouldn't be a line separating them, This is something we currently have in the Plasma-pa KCM (albeit with the wrong background color), but we lose it with this new KCM style.

zzag added a subscriber: zzag.Tue, Sep 10, 1:58 PM
zzag added inline comments.
src/qmlcontrols/kcmcontrols/qml/TabKCM.qml
154–161

FYI, QML has some features from ECMAScript 6, so you could make this code a bit cleaner, e.g.

for (const tab of tabs)
    tab.delegate.createObject(sl, {});

tabsRepeater.model = tabs.map(tab => tab.title);

(I didn't test whether this code actually works)

Also related: the lack of a real QML tab view: https://bugs.kde.org/show_bug.cgi?id=394296

  • Remove line beneath active tab

Nice, I think we need this.

From the image, I think the frame's background color needs to be the same as the active tab color, and there shouldn't be a line separating them, This is something we currently have in the Plasma-pa KCM (albeit with the wrong background color), but we lose it with this new KCM style.

Achieving this was easier than I initially thought, but now the color of the active tab doesn't match the content. With the line in this was much less of a problem

  • Implement Vlad's suggestion
nicolasfella edited the test plan for this revision. (Show Details)Tue, Sep 10, 2:23 PM
onvitaik added a subscriber: onvitaik.EditedTue, Sep 10, 3:04 PM

Can I also suggest making the tabs occupy the full width of the area available (sort of like Kate)? It minimizes the unused space and keeps everything in a nice rectangle:

Can I also suggest making the tabs occupy the full width of the area available (sort of like Kate)? It minimizes the unused space and keeps everything in a nice rectangle:

I kind of like it myself, but if we do this, we'd want to also make the same change for our QWidgets tabs for consistency. Might be better to do that later in subsequent patches.