Merge lp://qastaging/~unity-2d-team/unity-2d/unity-2d-dash-always-fullscreen-by-resolution into lp://qastaging/unity-2d

Proposed by Ugo Riboni
Status: Merged
Approved by: Florian Boucault
Approved revision: 875
Merged at revision: 875
Proposed branch: lp://qastaging/~unity-2d-team/unity-2d/unity-2d-dash-always-fullscreen-by-resolution
Merge into: lp://qastaging/unity-2d
Diff against target: 857 lines (+243/-267)
15 files modified
config.h.in (+6/-0)
data/com.canonical.Unity2d.gschema.xml (+10/-0)
libunity-2d-private/Unity2d/plugin.cpp (+6/-5)
libunity-2d-private/src/CMakeLists.txt (+0/-1)
libunity-2d-private/src/dashclient.cpp (+30/-0)
libunity-2d-private/src/dashclient.h (+9/-2)
libunity-2d-private/src/dashsettings.cpp (+0/-112)
libunity-2d-private/src/dashsettings.h (+0/-72)
panel/applets/appname/appnameapplet.cpp (+4/-1)
panel/applets/appname/windowhelper.cpp (+6/-51)
panel/applets/appname/windowhelper.h (+0/-1)
places/app/dashdeclarativeview.cpp (+1/-15)
places/app/dashdeclarativeview.h (+0/-1)
places/dash.qml (+2/-6)
tests/places/fullscreen.rb (+169/-0)
To merge this branch: bzr merge lp://qastaging/~unity-2d-team/unity-2d/unity-2d-dash-always-fullscreen-by-resolution
Reviewer Review Type Date Requested Status
Florian Boucault (community) Needs Fixing
Review via email: mp+89886@code.qastaging.launchpad.net

Description of the change

[dash][panel] Refactor testing for forced dash fullscreen based on resolution into DashClient

To post a comment you must log in.
Revision history for this message
Ugo Riboni (uriboni) wrote :
Revision history for this message
Florian Boucault (fboucault) wrote :

Any reason why not defining minimumSizeForDesktop as a property already?

Revision history for this message
Florian Boucault (fboucault) wrote :

Don't expose the constructor of DashClient. It is private for a reason, because DashClient is a singleton only accessed through ::instance(). You want it accessible from QML alright, expose what ::instance() gives you from the Unity2d QML plugin with "engine->rootContext()->setContextProperty(...)"

review: Needs Fixing
Revision history for this message
Florian Boucault (fboucault) wrote :

Otherwise looks great. Will carry slightly more detailed code inspection and functional review.

869. By Ugo Riboni

Expose DashClient as a singleton to QML

Revision history for this message
Lohith D Shivamurthy (dyams) wrote :

Functionally no issues found

Revision history for this message
Ugo Riboni (uriboni) wrote :

Pushed UI tests too

870. By Ugo Riboni

Merge changes from parent feature branch

871. By Ugo Riboni

Fix tests so that they check the right thing depending on current screen resolution (we decided it's not proper to force resolution changes during tests)

Revision history for this message
Ugo Riboni (uriboni) wrote :

As mentioned in commit message 871: the proper way to test that the dash is always full screen when the resolution is lower than a certain threshold is to change the resolution during the test.
However since we're expecting that people will run tests on their own machines, this was not deemed acceptable.
So we decided that as a compromise the tests would work differently based on the current resolution on the test machine.

In any case, I ran the tests both with a very small resolution and a big one and they pass in both cases.

872. By Ugo Riboni

Merge changes from parent branch

873. By Ugo Riboni

Merge changes from parent branch

874. By Ugo Riboni

Adapt the initial fullscreen test to current code

Revision history for this message
Ugo Riboni (uriboni) wrote :
875. By Ugo Riboni

Merge fixes to tests from parent branch

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches