Move "update" button from the left side to the right side in NIC module
ClosedPublic

Authored by schampailler on Aug 10 2018, 8:05 PM.

Details

Summary

The rationale of that change is that at some other place in the KInfoCenter, the buttons are aligned to the right (for example in the IEEE 1394 device info, see the "Generate 1394 Bus Reset" button), so I think it's better if everything is aligned the same way.

Before:

After:

Test Plan

Run the updated KInfoCenter. Go in the Network Information / Network Interfaces module and check that the update button is in the lower right corner instead of the left corner. The rest of the functionality is unchanged.

Diff Detail

Repository
R102 KInfoCenter
Lint
Lint Skipped
Unit
Unit Tests Skipped
schampailler created this revision.Aug 10 2018, 8:05 PM
Restricted Application added a project: Plasma. · View Herald TranscriptAug 10 2018, 8:05 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
schampailler requested review of this revision.Aug 10 2018, 8:05 PM
schampailler added a reviewer: ngraham.
schampailler edited the summary of this revision. (Show Details)Aug 10 2018, 8:08 PM
broulik added a subscriber: broulik.EditedAug 11 2018, 1:30 PM

Shouldn't it rather be called "Reload" with the appropriate icon?
Checkout KStandardAction or KGuiItem

I know you merely moved it but when you're touching it anyway... :)

schampailler added a comment.EditedAug 11 2018, 6:00 PM

I know you merely moved it but when you're touching it anyway... :)

thx for noting; my spare time is quite finite :-)

Now :

  • KGuiItem has nothing like refresh/update...
  • KStandardAction has "redisplay" (and I've no idea how it renders visually).

So should I try the latter ? (I ask because it's my first patch ever on KDE so I don't know anything about the API and its customs :-)

stF

ngraham requested changes to this revision.Aug 11 2018, 8:17 PM

What branch did you make this patch against? It doesn't apply for me against master:

error: while searching for:
       m_list->setHeaderLabels(columns);
       QHBoxLayout *hbox=new QHBoxLayout();
       box->addItem(hbox);
       m_updateButton=new QPushButton(i18n("&Update"),this);
       hbox->addWidget(m_updateButton);
       hbox->addStretch(1);
       QTimer* timer=new QTimer(this);
       timer->start(60000);
       connect(m_updateButton, &QPushButton::clicked, this, &KCMNic::update);

error: patch failed: Modules/nics/nic.cpp:95
Applying patch Modules/nics/nic.cpp with 1 reject...
Rejected hunk #1.

 Patch Failed! 
Usage Exception: Unable to apply patch!

Also, on the subject of improving the button itself (which I am fully in support of), that would be for another patch anyway.

Modules/nics/nic.cpp
98

Unrelated whitespace change; please revert.

102

Unrelated whitespace change; please revert.

This revision now requires changes to proceed.Aug 11 2018, 8:17 PM

removed modifications which were not 100% related to the patch, as proposed by @ngraham .

ngraham accepted this revision.Aug 16 2018, 9:54 PM
This revision is now accepted and ready to land.Aug 16 2018, 9:54 PM
schampailler retitled this revision from Move "update" button from the left side to the right side in NIC module to Move "update" button from the left side to the right side in NIC module.

Thanks for updating this with arcanist. Works like a dream now. :)

ngraham closed this revision.Aug 17 2018, 11:41 AM