Unity

Merge lp://qastaging/~azzar1/unity/fix-1028810 into lp://qastaging/unity

  1. fix-1028810
  2. Merge into trunk
Proposed by Andrea Azzarone
Status: Merged
Approved by: Omer Akram
Approved revision: no longer in the source branch.
Merged at revision: 2619
Proposed branch: lp://qastaging/~azzar1/unity/fix-1028810
Merge into: lp://qastaging/unity
Diff against target: 181 lines (+68/-11)
6 files modified
dash/DashView.cpp (+5/-2)
dash/FilterExpanderLabel.cpp (+11/-8)
dash/FilterExpanderLabel.h (+2/-0)
dash/FilterRatingsButton.cpp (+2/-1)
tests/autopilot/unity/emulators/dash.py (+20/-0)
tests/autopilot/unity/tests/test_dash.py (+28/-0)
To merge this branch: bzr merge lp://qastaging/~azzar1/unity/fix-1028810
Reviewer Review Type Date Requested Status
Brandon Schaefer (community) Approve
Tim Penhey (community) Needs Information
Francis Ginther Abstain
jenkins (community) continuous-integration Needs Fixing
Review via email: mp+116744@code.qastaging.launchpad.net

Commit message

Fix bottom-up key navigation in dash filterbar.

Description of the change

== Problem ==
Bottom - Up key navigation is broken in dash filterbar

== Test ==
AP test added.

Requires lp:~andyrock/nux/fix-gridhlayout-bu-keynav.

To post a comment you must log in.
Revision history for this message
jenkins (martin-mrazik+qa) wrote :

FAILED: Continuous integration, rev:2518
http://jenkins.qa.ubuntu.com/job/unity-ci/123/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=amd64/123/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=i386/123/console

review: Needs Fixing (continuous-integration)
Revision history for this message
Martin Mrazik (mrazik) wrote :

Sorry for the jenkins failure. I'm looking into this.

Revision history for this message
Francis Ginther (fginther) wrote :

Review was claimed by accident, please ignore.

review: Abstain
Revision history for this message
Tim Penhey (thumper) wrote :

  FilterExpanderLabel* filter = dynamic_cast<FilterExpanderLabel*>(child);
  if (child)
    tabs.push_back(filter->expander_view());

Did you mean "if (filter)" ?

review: Needs Information
Revision history for this message
Andrea Azzarone (azzar1) wrote :

> FilterExpanderLabel* filter = dynamic_cast<FilterExpanderLabel*>(child);
> if (child)
> tabs.push_back(filter->expander_view());
>
> Did you mean "if (filter)" ?

Yes. Fixing.

Revision history for this message
Andrea Azzarone (azzar1) wrote :

Fixed.

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Could you split up that AP tests into smaller ones? Or split some of the logic into smaller ones so it will be easy to figure out what is causing a potential problem later on.

Such as making a test that proves that it is getting to Filter Results, then a test that shows it can tab to the first filter result ... etc.

review: Needs Fixing
Revision history for this message
Andrea Azzarone (azzar1) wrote :

> Could you split up that AP tests into smaller ones? Or split some of the logic
> into smaller ones so it will be easy to figure out what is causing a potential
> problem later on.

Hey I missed this comment, sorry!

>
> Such as making a test that proves that it is getting to Filter Results, then a
> test that shows it can tab to the first filter result ... etc.

We already have these tests.

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Awesome then! Looks good!

review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/1114/console reported an error when processing this lp:~andyrock/unity/fix-1028810 branch.
Not merging it.

Revision history for this message
Unity Merger (unity-merger) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/1115/console reported an error when processing this lp:~andyrock/unity/fix-1028810 branch.
Not merging it.

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.