Merge lp:~aacid/unity8/scopeSearchHintText into lp:unity8

Proposed by Albert Astals Cid
Status: Merged
Approved by: Michael Zanetti
Approved revision: 1085
Merged at revision: 1144
Proposed branch: lp:~aacid/unity8/scopeSearchHintText
Merge into: lp:unity8
Prerequisite: lp:~aacid/unity8/scopeActivatePreview
Diff against target: 58 lines (+15/-1)
4 files modified
qml/Components/PageHeader.qml (+1/-0)
qml/Dash/GenericScopeView.qml (+1/-0)
tests/mocks/Unity/fake_scope.cpp (+1/-1)
tests/qmltests/Dash/tst_DashContent.qml (+12/-0)
To merge this branch: bzr merge lp:~aacid/unity8/scopeSearchHintText
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Michael Zanetti (community) Approve
Review via email: mp+228655@code.launchpad.net

This proposal supersedes a proposal from 2014-07-25.

Commit message

Pass the scope search hint up to the search line

Description of the change

* Are there any related MPs required for this MP to build/function as expected?
https://code.launchpad.net/~unity-team/unity-scopes-shell/catch-no-search-hint/+merge/228288
No, but there is a crash somewhere in scopes plugin that makes it crash for some scopes and also we have a white on white text problem, so it eihter needs the SDK team to fix the color on SuruDark or the Dash being a separate app so we can not use SuruDark theme

 * Did you perform an exploratory manual test run of your code change and any related functionality?
Yes, changed hint color of /usr/lib/x86_64-linux-gnu/qt5/qml/Ubuntu/Components/TextField.qml to be red manually and it is showing up fine

 * Did you make sure that your branch does not contain spurious tags?
Yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

 * If you changed the UI, has there been a design review?
This is requested by design

To post a comment you must log in.
Revision history for this message
Michael Zanetti (mzanetti) wrote : Posted in a previous version of this proposal

I think we could add a simple test here to make sure the value from the scope ends up in the placeholderText.

review: Needs Fixing
Revision history for this message
Michael Zanetti (mzanetti) wrote : Posted in a previous version of this proposal

Otherwise it works fine except the above mentioned issues. I tried to merge it into the dash-as-app branch but that makes me hit the mentioned crash.

Revision history for this message
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

> I think we could add a simple test here to make sure the value from the scope
> ends up in the placeholderText.

Added.

Revision history for this message
Michael Zanetti (mzanetti) wrote : Posted in a previous version of this proposal

thanks. looks good.

 * Did you perform an exploratory manual test run of the code change and any related functionality?

yeah man!

 * Did CI run pass? If not, please explain why.

CI? what's that?

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

reapproving.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~aacid/unity8/scopeSearchHintText updated
1086. By Albert Astals Cid

Merge

1087. By Albert Astals Cid

Merge

1088. By Albert Astals Cid

Merge

1089. By Albert Astals Cid

Merge

1090. By Albert Astals Cid

Merge

1091. By Albert Astals Cid

Merge

1092. By Albert Astals Cid

Merge

1093. By Albert Astals Cid

Merge

1094. By Albert Astals Cid

Merge

1095. By Albert Astals Cid

Merge

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'qml/Components/PageHeader.qml'
--- qml/Components/PageHeader.qml 2014-08-08 09:21:35 +0000
+++ qml/Components/PageHeader.qml 2014-08-08 09:21:36 +0000
@@ -32,6 +32,7 @@
32 property bool searchEntryEnabled: false32 property bool searchEntryEnabled: false
33 property ListModel searchHistory: SearchHistoryModel33 property ListModel searchHistory: SearchHistoryModel
34 property alias searchQuery: searchTextField.text34 property alias searchQuery: searchTextField.text
35 property alias searchHint: searchTextField.placeholderText
35 property alias showSignatureLine: bottomBorder.visible36 property alias showSignatureLine: bottomBorder.visible
3637
37 property alias bottomItem: bottomContainer.children38 property alias bottomItem: bottomContainer.children
3839
=== modified file 'qml/Dash/GenericScopeView.qml'
--- qml/Dash/GenericScopeView.qml 2014-08-08 09:21:35 +0000
+++ qml/Dash/GenericScopeView.qml 2014-08-08 09:21:36 +0000
@@ -420,6 +420,7 @@
420 objectName: "scopePageHeader"420 objectName: "scopePageHeader"
421 width: parent.width421 width: parent.width
422 title: scopeView.scope ? scopeView.scope.name : ""422 title: scopeView.scope ? scopeView.scope.name : ""
423 searchHint: scopeView.scope && scopeView.scope.searchHint || i18n.tr("Search")
423 showBackButton: scopeView.hasBackAction424 showBackButton: scopeView.hasBackAction
424 searchEntryEnabled: true425 searchEntryEnabled: true
425 scopeStyle: scopeView.scopeStyle426 scopeStyle: scopeView.scopeStyle
426427
=== modified file 'tests/mocks/Unity/fake_scope.cpp'
--- tests/mocks/Unity/fake_scope.cpp 2014-08-08 09:21:35 +0000
+++ tests/mocks/Unity/fake_scope.cpp 2014-08-08 09:21:36 +0000
@@ -69,7 +69,7 @@
6969
70QString Scope::searchHint() const70QString Scope::searchHint() const
71{71{
72 return QString("");72 return QString("Search %1").arg(m_name);
73}73}
7474
75QString Scope::shortcut() const75QString Scope::shortcut() const
7676
=== modified file 'tests/qmltests/Dash/tst_DashContent.qml'
--- tests/qmltests/Dash/tst_DashContent.qml 2014-08-08 09:21:35 +0000
+++ tests/qmltests/Dash/tst_DashContent.qml 2014-08-08 09:21:36 +0000
@@ -380,5 +380,17 @@
380 mouseClick(allButton, 0, 0);380 mouseClick(allButton, 0, 0);
381 tryCompare(dashNavigation.currentNavigation, "navigationId", "middle2");381 tryCompare(dashNavigation.currentNavigation, "navigationId", "middle2");
382 }382 }
383
384 function test_searchHint() {
385 var dashContentList = findChild(dashContent, "dashContentList");
386 verify(dashContentList !== null);
387 var scope = findChild(dashContent, "MockScope1 loader");
388 waitForRendering(scope);
389
390 var categoryListView = findChild(scope, "categoryListView");
391 waitForRendering(categoryListView);
392
393 compare(categoryListView.pageHeader.searchHint, "Search People");
394 }
383 }395 }
384}396}

Subscribers

People subscribed via source and target branches