D12855: Move Widget search field to its own row so it doesn't get compressed

[Nathaniel Graham]

ngraham added a comment.


  A few more code comments that I missed in the first go-around, sorry. Behavior-wise, I think this is a huge improvement!

INLINE COMMENTS

> WidgetExplorer.qml:155
>      }
> +
>      /*

whitespace change :)

> WidgetExplorer.qml:236
> +                } else {
> +                     newSearchRow.height = units.smallSpacing * 5
>              }

Could this be expressed in terms of the search field's actual height? That way we wouldn't be vulnerable to height issues if the `units.smallSpacing value` changed at some point.

> WidgetExplorer.qml:265
> +
> +            height: 0                   
> +

Is this needed?

> sharvey wrote in WidgetExplorer.qml:157
> I'm always unclear what to do when I find blocks of other people's comments. Some reviewers absolutely loathe comments being left in; others don't seem to object as much. I've asked for a comment policy to be put together as part of the new-onboarding initiative. I was taught to comment my code for clarity, but not to leave blocks of work-in-progress code like this behind. I hesitated to take it out in case someone was planning on returning to it, but I probably shouldn't have added the graffiti. I'll remove my unnecessary side comment.

In general, KDE policy is to remove code rather than commenting it out. But we also try to encourage patches to be as focused as possible. Hence, what I think makes sense is to ignore the commented-out block in this patch, and remove it entirely in another one.

> ngraham wrote in WidgetExplorer.qml:389
> Unrelated whitespace change

Looks like it's still there? Not a huge deal, but it would be good to figure out why your changes keep adding or removing whitespace.

REPOSITORY
  R119 Plasma Desktop

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

To: sharvey, ngraham, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180514/5a26f279/attachment-0001.html>