Unity 8

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2013-11-29
Michał Sawicz		2013-11-14	Needs Fixing on 2013-11-22
Albert Astals Cid (community)			Needs Information on 2013-11-22
Review via email: mp+195203@code.launchpad.net

This proposal has been superseded by a proposal from 2014-01-30.

Commit message

Changed category transition animation to expand and collapse at a constant speed.

lp:~nicolas-doffay/unity8/category-transition-speed-fix updated on 2013-11-14

522. By Nicolas d'Offay on 2013-11-14: Wrapped onStarted logic in brackets.

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-11-14:

That's not enough, I'm afraid.

The intermediate height is more complicated: it needs to take y and uncollapsed height into account. Say the category is scrolled up so that its first row is offscreen, you animate its height to genericScopeHeight, but that means the last row of the expanded category won't get animated (just scroll a category up so that it's offscreen and expand it then - you'll see the bottom behaving incorrectly). If uncollapsedHeight is smaller than genericScopeHeight, you animate to genericScopeHeight and then collapse back to uncollapsedHeight (you can see that when expanding "Applications" in Home - the second category disappears for a fraction of a second).

review: Needs Fixing

Revision history for this message

Albert Astals Cid (aacid) wrote on 2013-11-14:

Don't call the variable genericScopeHeight, call it height animation limit or something, the filtergrid knows nothing about scopes

review: Needs Fixing

lp:~nicolas-doffay/unity8/category-transition-speed-fix updated on 2013-11-14

523. By Nicolas d'Offay on 2013-11-14: Catered for transitions where the uncollapased height is less than the height.
524. By Nicolas d'Offay on 2013-11-14: Renamed height variable.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-17:

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity8-ci/1625/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-17:

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity8-ci/1638/rebuild

review: Approve (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2013-11-20:

There's something wrong, see how expanding the Apps category on Home scope on the desktop gives you a blinking Files&Folders movement. That doesn't happen without this.

review: Needs Fixing

lp:~nicolas-doffay/unity8/category-transition-speed-fix updated on 2013-11-20

525. By Nicolas d'Offay on 2013-11-20: Fixed transitioning bug.

Revision history for this message

Albert Astals Cid (aacid) wrote on 2013-11-20:

I'd appreciate if you could add a unittests, do you think this is feasible?

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-20:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1683/rebuild

review: Needs Fixing (continuous-integration)

lp:~nicolas-doffay/unity8/category-transition-speed-fix updated on 2013-11-21

526. By Nicolas d'Offay on 2013-11-21: Updated FilterGrid test to use animation system.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-21:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1691/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2013-11-22:

Doesn't seem constant speed to me when doing this on the desktop:
* Change to Applications scope
* I have 3 categories (installed, suggestions, dash plugins)
* Scroll to bottom (suggestions is almost at the top)
* Expand the suggestions category
* Wait
* Unexpand the suggestions category
* Wait

My impression is that expansion and unexpansion don't happen at the same speed. I'd appreciate if someone else could comment.

review: Needs Information

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-11-22:

Yeah, it feels like collapsing happens a lot faster than expansion.

One more thing - the scroll and expansion animations are not in sync:

* go to top of Applications
* expand More suggestions

You can see the Dash plugins category for a split second.

review: Needs Fixing

Revision history for this message

Nicolas d'Offay (nicolas-doffay) wrote on 2013-11-27:

Variable speeds are the only way to get around a noticeable "bounce" if the height is set to the scopeViewHeight then set to collapsed/uncollapsed height onStopped.

lp:~nicolas-doffay/unity8/category-transition-speed-fix updated on 2013-11-27

527. By Nicolas d'Offay on 2013-11-27: Attempted fix for speed issue.
528. By Nicolas d'Offay on 2013-11-27: Fixed up comparison operator.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-27:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1753/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-11-27:

Aborted due to bug #1255578.

Revision history for this message

Albert Astals Cid (aacid) wrote on 2013-11-28:

Hmmm, i still think there's something weird with the previous scenario i mentioned.

Also in this scenario:
* Change to Applications scope
* I have 3 categories (installed, suggestions, dash plugins)
* Expand the suggestions category
* Wait
* Now scroll back to the initial position (so that suggestions is still expanded but you actually only see the same you saw when it was unexpanded)
* Unexpand the suggestions category

I undestand the unexpansion in this case should be immediate since we're not going to see the animation happen, but still takes some time to change the ^ marker

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-29:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1778/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-29:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1785/rebuild

review: Needs Fixing (continuous-integration)

Unmerged revisions

528. By Nicolas d'Offay on 2013-11-27: Fixed up comparison operator.
527. By Nicolas d'Offay on 2013-11-27: Attempted fix for speed issue.
526. By Nicolas d'Offay on 2013-11-21: Updated FilterGrid test to use animation system.
525. By Nicolas d'Offay on 2013-11-20: Fixed transitioning bug.
524. By Nicolas d'Offay on 2013-11-14: Renamed height variable.
523. By Nicolas d'Offay on 2013-11-14: Catered for transitions where the uncollapased height is less than the height.
522. By Nicolas d'Offay on 2013-11-14: Wrapped onStarted logic in brackets.
521. By Nicolas d'Offay on 2013-11-14: Added two step animation for category expansion and collapse.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Mohammad Rakibul Hasan

Nicolas d'Offay

Unity Team

 === modified file 'Components/FilterGrid.qml'
 --- Components/FilterGrid.qml	2013-10-15 12:28:14 +0000
 +++ Components/FilterGrid.qml	2013-11-27 14:01:33 +0000
@@ -71,22 +71,53 @@
      NumberAnimation {
          property bool filterEndValue
++        property real filterGridAnimationHeight
          id: filterAnimation
          target: root
          property: "height"
--        to: filterEndValue ? root.collapsedHeight : root.uncollapsedHeight
++        to: {
++            //Collapse
++            if (filterEndValue) {
++                if (root.collapsedHeight < filterGridAnimationHeight) {
++                    return root.collapsedHeight;
++                } else {
++                    return filterGridAnimationHeight;
++                }
++            }
++            //Expand
++            else {
++                if (root.uncollapsedHeight < filterGridAnimationHeight) {
++                    return root.uncollapsedHeight;
++                } else {
++                    return filterGridAnimationHeight;
++                }
++            }
++        }
++
          // Duration and easing here match the ListViewWithPageHeader::m_contentYAnimation
          // otherwise since both animations can run at the same time you'll get
          // some visual weirdness.
          duration: 200
          easing.type: Easing.InOutQuad
          onStopped: {
++            if (root.height === filterGridAnimationHeight) {
++                //Collapse
++                if (filterEndValue) {
++                    root.height = root.collapsedHeight;
++                }
++                //Expand
++                else {
++                    root.height = root.uncollapsedHeight;
++                }
++            }
++
              root.filter = filterEndValue;
+         }
+     }
--    function startFilterAnimation(filter) {
--        filterAnimation.filterEndValue = filter
++    function startFilterAnimation(filter, scopeViewHeight) {
++        filterAnimation.filterGridAnimationHeight = scopeViewHeight;
++        filterAnimation.filterEndValue = filter;
          filterAnimation.start();
+     }
 === modified file 'Dash/GenericScopeView.qml'
 --- Dash/GenericScopeView.qml	2013-11-19 14:33:40 +0000
 +++ Dash/GenericScopeView.qml	2013-11-27 14:01:33 +0000
@@ -226,11 +226,7 @@
                                  var shrinkingVisible = shouldFilter && y + item.collapsedHeight < categoryView.height;
                                  var growingVisible = !shouldFilter && y + height < categoryView.height;
                                  if (!previewListView.open || !shouldFilter) {
--                                    if (shrinkingVisible || growingVisible) {
--                                        item.startFilterAnimation(shouldFilter)
--                                    } else {
--                                        item.filter = shouldFilter;
--                                    }
++                                    item.startFilterAnimation(shouldFilter, scopeView.height)
                                      if (!shouldFilter && !previewListView.open) {
                                          categoryView.maximizeVisibleArea(index, item.uncollapsedHeight);
+                                     }
 === modified file 'tests/qmltests/Components/tst_FilterGrid.qml'
 --- tests/qmltests/Components/tst_FilterGrid.qml	2013-08-09 07:41:12 +0000
 +++ tests/qmltests/Components/tst_FilterGrid.qml	2013-11-27 14:01:33 +0000
@@ -49,6 +49,10 @@
              CheckBox {
                  id: filterCheckBox
                  checked: true
++
++                onCheckedChanged: {
++                    filterGrid.startFilterAnimation(!checked, filterGrid.height);
++                }
+             }
+         }
+     }
@@ -84,7 +88,7 @@
              maximumNumberOfColumns: 3
              collapsedRowCount:
                  collapsedRowCountSelector.values[collapsedRowCountSelector.selectedIndex]
--            filter: filterCheckBox.checked
++            filter: false
              minimumHorizontalSpacing: units.gu(1)
              delegateWidth: units.gu(6)
              delegateHeight: units.gu(6)
@@ -109,15 +113,16 @@
          name: "FilterGrid"
          when: windowShown
++        function initTestCase() {
++            filterCheckBox.checked = false;
++        }
++
          function test_turningFilterOffShowsAllElements() {
++            filterCheckBox.checked = false
              tryCompareFunction(countVisibleDelegates, 6)
--            filterCheckBox.checked = false
--
++            filterCheckBox.checked = true
              tryCompareFunction(countVisibleDelegates, 12)
--
--            // back to initial state
--            filterCheckBox.checked = true
+         }
          function test_collapsedRowCount() {

Unity 8

Merge lp:~nicolas-doffay/unity8/category-transition-speed-fix into lp:unity8

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers