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

Nathaniel Graham noreply at phabricator.kde.org
Mon May 14 02:43:37 UTC 2018


ngraham added inline comments.

INLINE COMMENTS

> WidgetExplorer.qml:157
>      /*
> +    * This was here when I got here 5/12/18 - sharvey
>      PlasmaCore.Dialog {

? Not sure The extra comment adds anything. Instead we should just remove the commented-out code in a new patch IMHO.

> WidgetExplorer.qml:203
>          id: topBar
> +        spacing: 10
>          anchors {

Can we use some multiple of `units.smallSpacing` or something here instead of a hardcoded pixel value?

> WidgetExplorer.qml:210
>  
> +
>          Item {

Unrelated whitespace change

> WidgetExplorer.qml:215
>              Layout.fillWidth: true
> -            Layout.minimumHeight: Math.max(heading.height, searchInput.height)
> +            Layout.minimumHeight: heading.height
>              Layout.alignment: Qt.AlignVCenter

Does anything break if we just remove this line entirely?

> WidgetExplorer.qml:223
>                  elide: Text.ElideRight
> -                visible: !header.showingSearch
> -            }
> -            PlasmaComponents.TextField {
> -                id: searchInput
> -                width: parent.width
> -                clearButtonShown: true
> -                anchors.verticalCenter: parent.verticalCenter
> -                placeholderText: i18nd("plasma_shell_org.kde.plasma.desktop", "Search...")
> -                onTextChanged: {
> -                    list.positionViewAtBeginning()
> -                    list.currentIndex = -1
> -                    widgetExplorer.widgetsModel.searchTerm = text
> -                    header.showingSearch = (text != "");
> -                }
> -
> -                Component.onCompleted: forceActiveFocus()
> -                visible: header.showingSearch
> +                visible: true
>              }

No need for this anymore, since `True` is the default value.

> WidgetExplorer.qml:263
>  
> +        // move search textbox to its own row
> +        RowLayout {

This comment, if necessary at all, should be phrased in terms of the current state, not the change that added it.

> WidgetExplorer.qml:274
> +
> +            spacing: 10
> +

Ditto; let's not use a hardcoded pixel value.

> WidgetExplorer.qml:389
>  }
> -

Unrelated whitespace change

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/b1722426/attachment.html>


More information about the Plasma-devel mailing list