D7087: Add "Copy Info" button to the About System KCM

[Henrik Fehlauer]

rkflx added a comment.


  In D7087#284846 <https://phabricator.kde.org/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 <https://phabricator.kde.org/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 <https://phabricator.kde.org/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

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

You could add a `const`.

> Module.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…

> gregormi 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180630/220b8691/attachment.html>