View Revision
rkflx added a comment.

In D7087#284846, @gregormi wrote:

I fixed the layout and it looks now like this in Qt Designer

Awesome, I guess you got the overall gist ;)

The blue arrow shows a difference to your proposal. Do you think the grid layout there is ok?

I probably used a vertical layout for this (IIRC), which should work the same but is a bit simpler. Same thing for the purely horizontal layout at the bottom, where you also used a grid. Again, this does not really matter that much.

Nevertheless, somehow there seems to be a glitch near KernelName, so the values for both kernel and architecture are displaced a bit to the bottom. When I break the layout and then redo it, it works fine for me. Not sure how you got into that situation ;) Let me know if I should upload my version if you cannot get it fixed with your Qt Designer.


In D7087#284855, @gregormi wrote:

I added some qDebug code in Module::copyToClipboard()
[…]
Any idea why the slot method is called twice?

Module::load() is called twice (see comment in constructor or D2300), so you have two connections. In general you could use a Qt::UniqueConnection in your connect to avoid this, but I think here it's actually better to move your setup code (i.e. everything except for labelsForClipboard.clear()) to the constructor instead.


INLINE COMMENTS
View InlineModule.cpp:161
auto dummyDistroDescriptionLabel = new QLabel(i18nc("@title:row", "Distro:"), this);
labelsForClipboard << qMakePair(dummyDistroDescriptionLabel, ui->nameVersionLabel);

You could add a const.


View InlineModule.cpp:263
// note that this loop does not necessarily represent the same order as in the GUI
for (auto p : qAsConst(labelsForClipboard)) {
auto descriptionLabelText = p.first->text();

Coming back to this after a month, I now wonder what p stands for, which might indicate that variable could get a better name…


View Inlinegregormi wrote in Module.cpp:260

Good idea. DONE.

I'm afraid you missed the "hidden" part, so it shows up right in front of the distro logo ;)

Adding

dummyDistroDescriptionLabel->hide();

where you are creating the label solves the issue for me.


REPOSITORY
R102 KInfoCenter

REVISION DETAIL
https://phabricator.kde.org/D7087

To: gregormi, ngraham, dhaumann, rkflx
Cc: rkflx, dhaumann, ltoscano, sebas, elvisangelaccio, cfeck, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart