Use consistent highlight
ClosedPublic

Authored by niccolove on Wed, Mar 4, 9:37 AM.

Details

Summary

Use the PlasmaComponents.Highlight feature to be consistent with other lists.

Diff Detail

Repository
R116 Plasma Network Management Applet
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
niccolove created this revision.Wed, Mar 4, 9:37 AM
Restricted Application added a project: Plasma. · View Herald TranscriptWed, Mar 4, 9:37 AM
Restricted Application added a reviewer: jgrulich. · View Herald Transcript
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
niccolove requested review of this revision.Wed, Mar 4, 9:37 AM
ngraham added a subscriber: ngraham.Wed, Mar 4, 3:20 PM

This needs a rebase. Also you probably need to delete the existing mouseover highlight effect.

This needs a rebase. Also you probably need to delete the existing mouseover highlight effect.

Git is telling me this is up to date. Also, what existing mouseover effect?

niccolove updated this revision to Diff 77001.Thu, Mar 5, 10:20 AM

Remove old highlight

Ah, this one.

niccolove updated this revision to Diff 77003.Thu, Mar 5, 10:25 AM

Right margin of button

ngraham requested changes to this revision.Thu, Mar 5, 5:44 PM
ngraham added inline comments.
applet/contents/ui/ConnectionItem.qml
56

ConnectionItem.qml already has onContainsMouseChanged (at the bottom of the file); add the new stuff there.

This revision now requires changes to proceed.Thu, Mar 5, 5:44 PM
niccolove updated this revision to Diff 77203.Sun, Mar 8, 2:19 PM

Fixed double declaration

niccolove marked an inline comment as done.Sun, Mar 8, 2:19 PM

Now it needs a rebase, as there are merge conflicts. :)

you need to rebase on top of master from the remote. This looks to have been branched off of the state of master from last November or something.

Try git pull --rebase origin master That should give you some merge conflicts that you can fix.

niccolove updated this revision to Diff 77214.Sun, Mar 8, 3:49 PM

rebase x2

davidre added a subscriber: davidre.Sun, Mar 8, 3:52 PM

Probably your arc diff base target thingy didn't switch and is on the old commit so this includes everything new on the master branch. Specify it manually with arc diff master or arc diff origin/master

niccolove updated this revision to Diff 77215.Sun, Mar 8, 3:53 PM

rebase x3

There we go, much better now. :) Reviewing...

Does this work for you? I still see the old highlight.

niccolove updated this revision to Diff 77217.Sun, Mar 8, 4:12 PM

remove old highlight

niccolove updated this revision to Diff 77219.Sun, Mar 8, 4:14 PM

remove old highlight

Should now work?

Now I see no highlight effect at all.

Does this work for you?

niccolove updated this revision to Diff 77253.Mon, Mar 9, 8:44 AM

fixed name of list

Sorry, I had different files with different versions. Now it shows correctly for me, with all files up to date. That said, is it normal that:

  • the top elements has a top separator
  • icon size of the second element is smaller

? seems wrong to me, but not because of this patch

ngraham accepted this revision.Mon, Mar 9, 3:08 PM

Thanks, everything works now. :)

I don't see that extra line in mine. But yeah, if it's reproducible, it's a bug that should be fixed.

jgrulich accepted this revision.Mon, Mar 9, 3:13 PM
This revision is now accepted and ready to land.Mon, Mar 9, 3:13 PM
This revision was automatically updated to reflect the committed changes.
gvgeo added a subscriber: gvgeo.Tue, Mar 10, 9:44 AM

Couple of notes, as I'm not sure what the aim was here. I have not test it, take them with a grain of salt.

  • Highlight area was changed, without changing the click area.
  • Connect button pop up area change, was not documented.
  • Highlight svg is not consistent with list's highlight. And maybe not always look good. (Although it's designation is pressed, everywhere used as highlight, which is an different story itself.)
  • There is code left in various places, listitem.qml possibly could be removed completely now.

The line above, is because you did not build plasma-nm but instead copied the qml files.
See T10470 for the icon size.