Merge lp://staging/~zsombi/ubuntu-ui-toolkit/bottomEdge into lp://staging/ubuntu-ui-toolkit/staging

Proposed by Zsombor Egri
Status: Merged
Approved by: Tim Peeters
Approved revision: 1781
Merged at revision: 1731
Proposed branch: lp://staging/~zsombi/ubuntu-ui-toolkit/bottomEdge
Merge into: lp://staging/ubuntu-ui-toolkit/staging
Diff against target: 4292 lines (+3826/-40)
47 files modified
components.api (+48/-0)
examples/ubuntu-ui-toolkit-gallery/BottomEdgePage.qml (+177/-14)
examples/ubuntu-ui-toolkit-gallery/Template.qml (+1/-0)
examples/ubuntu-ui-toolkit-gallery/TemplateRow.qml (+1/-1)
examples/ubuntu-ui-toolkit-gallery/gallery-logging.config (+1/-0)
src/Ubuntu/Components/1.3/AdaptivePageLayout.qml (+0/-2)
src/Ubuntu/Components/1.3/PageTreeNode.qml (+0/-4)
src/Ubuntu/Components/ComponentModule.pro (+1/-1)
src/Ubuntu/Components/Themes/Ambiance/1.3/BottomEdgeStyle.qml (+125/-0)
src/Ubuntu/Components/Themes/Ambiance/Ambiance.pro (+2/-1)
src/Ubuntu/Components/Themes/Ambiance/qmldir (+2/-0)
src/Ubuntu/Components/plugin/plugin.cpp (+6/-0)
src/Ubuntu/Components/plugin/plugin.pri (+9/-2)
src/Ubuntu/Components/plugin/ucaction.h (+6/-5)
src/Ubuntu/Components/plugin/ucbottomedge.cpp (+1059/-0)
src/Ubuntu/Components/plugin/ucbottomedge.h (+120/-0)
src/Ubuntu/Components/plugin/ucbottomedge_p.h (+111/-0)
src/Ubuntu/Components/plugin/ucbottomedgehint.cpp (+1/-1)
src/Ubuntu/Components/plugin/ucbottomedgehint.h (+2/-0)
src/Ubuntu/Components/plugin/ucbottomedgeregion.cpp (+253/-0)
src/Ubuntu/Components/plugin/ucbottomedgeregion.h (+78/-0)
src/Ubuntu/Components/plugin/ucbottomedgestyle.cpp (+65/-0)
src/Ubuntu/Components/plugin/ucbottomedgestyle.h (+60/-0)
src/Ubuntu/Components/plugin/ucstyleditembase.cpp (+21/-5)
src/Ubuntu/Components/plugin/ucstyleditembase.h (+1/-0)
src/Ubuntu/Components/plugin/ucstyleditembase_p.h (+2/-0)
src/Ubuntu/Test/plugin/uctestextras.cpp (+136/-3)
src/Ubuntu/Test/plugin/uctestextras.h (+6/-0)
tests/unit_x11/tst_bottomedge/AddCustomRegionOnCompleted.qml (+41/-0)
tests/unit_x11/tst_bottomedge/AddCustomRegionOwnedByOtherBottomEdge.qml (+39/-0)
tests/unit_x11/tst_bottomedge/AddCustomRegionUsingDataProperty.qml (+33/-0)
tests/unit_x11/tst_bottomedge/AddCustomRegionUsingRegionsProperty.qml (+33/-0)
tests/unit_x11/tst_bottomedge/AlternateDefaultRegionContent.qml (+53/-0)
tests/unit_x11/tst_bottomedge/AlternateRegionContent.qml (+47/-0)
tests/unit_x11/tst_bottomedge/AutoCollapseInPageHeader.qml (+45/-0)
tests/unit_x11/tst_bottomedge/AutoCollapseInPageWithPageHeader.qml (+44/-0)
tests/unit_x11/tst_bottomedge/BottomEdgeInItem.qml (+36/-0)
tests/unit_x11/tst_bottomedge/ClearCustomRegions.qml (+42/-0)
tests/unit_x11/tst_bottomedge/Defaults.qml (+28/-0)
tests/unit_x11/tst_bottomedge/DifferentSizes.qml (+34/-0)
tests/unit_x11/tst_bottomedge/LastItem.qml (+46/-0)
tests/unit_x11/tst_bottomedge/LeanActiveRegionChange.qml (+45/-0)
tests/unit_x11/tst_bottomedge/OverlappingRegions.qml (+46/-0)
tests/unit_x11/tst_bottomedge/ShorterBottomEdge.qml (+39/-0)
tests/unit_x11/tst_bottomedge/tst_bottomedge.cpp (+856/-0)
tests/unit_x11/tst_bottomedge/tst_bottomedge.pro (+23/-0)
tests/unit_x11/unit_x11.pro (+2/-1)
To merge this branch: bzr merge lp://staging/~zsombi/ubuntu-ui-toolkit/bottomEdge
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Tim Peeters Approve
Review via email: mp+278336@code.staging.launchpad.net

Commit message

BottomEdge component

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Tim Peeters (tpeeters) wrote :

In the gallery, I can over-drag the bottom edge a bit, which puts the app in a weird state until it pops back to a closed bottom edge in about 500ms. See https://www.dropbox.com/s/f1n2cthukzl4e31/Screenshot%202015-11-23%2021.14.55.png?dl=0

I think instead of this, the contents should snap to open fully instead of being able to drag it too far.

review: Needs Fixing
Revision history for this message
Tim Peeters (tpeeters) wrote :

qml: WARNING! Do not put Page/Tabs/PageStack inside another Page because that causes confusion which is the active page that sets the title and actions.

This is a bit annoying. I think we can remove this warning now. With the new header, it is no longer a problem to define a Page inside another Page.

Revision history for this message
Tim Peeters (tpeeters) wrote :

> qml: WARNING! Do not put Page/Tabs/PageStack inside another Page because that
> causes confusion which is the active page that sets the title and actions.
>
> This is a bit annoying. I think we can remove this warning now. With the new
> header, it is no longer a problem to define a Page inside another Page.

better to do that in another MR, so just leave it here.

Revision history for this message
Tim Peeters (tpeeters) wrote :

I find the line on top of the bottom edge when it is open ugly.

https://www.dropbox.com/s/mvexgb66nt5nr3n/Screenshot%202015-11-23%2021.27.24.png?dl=0

Also, see the comments I added to the API doc, and one diff comment below (more to follow).

Revision history for this message
Tim Peeters (tpeeters) wrote :

I don't understand what "push content into the layout" does, and why when I open the bottom edge with that checkbox checked, it says state: hidden in the header: https://www.dropbox.com/s/dns1zeuivsdfn49/Screenshot%202015-11-23%2021.29.50.png?dl=0

Revision history for this message
Tim Peeters (tpeeters) wrote :

See more inline comments.

Since the BottomEdgeRegions are (at the moment) mainly used in the browser, perhaps we should ask the browser guys to test this MR (ideally to have a prototype browser that uses this) to see if it fulfills all their requirements.

Revision history for this message
Zsombor Egri (zsombi) wrote :

> See more inline comments.
>
>
> Since the BottomEdgeRegions are (at the moment) mainly used in the browser,
> perhaps we should ask the browser guys to test this MR (ideally to have a
> prototype browser that uses this) to see if it fulfills all their
> requirements.

It is not only in the Browser, there can be many other use cases for it. And then the whole logic is implemented using that. We talked about this in Berlin with mzanetti while going to the hotel from airport that he'd need such a functionality in his apps. And honestly I would not spend time to add it later when we have it right away.

Revision history for this message
Zsombor Egri (zsombi) wrote :

Replied to the inline comments, and pushed the changes I had so far. Revisions starting 1740 are for tests, please check them one by one to see the progress.

There are more tests to come. Will keep you posted.

1740. By Zsombor Egri

implicit height fix

1741. By Zsombor Egri

implicit height fix

1742. By Zsombor Egri

complete default check

1743. By Zsombor Egri

fix reparenting

1744. By Zsombor Egri

adding remaining test skeletons

1745. By Zsombor Egri

test that BottomEdgeStyle instance is always the last child

1746. By Zsombor Egri

test commit on click

1747. By Zsombor Egri

touch-click commits

1748. By Zsombor Egri

rename state into status, content into contentUrl

1749. By Zsombor Egri

verifying state transition when the bottom edge is swiped for a short time

1750. By Zsombor Egri

verifying state transition when the bottom edge is swiped for a short time

1751. By Zsombor Egri

add some threshold and extra steps especially for touch to properly produce a swipe

1752. By Zsombor Egri

drag downwards after dragged upwards collapses

1753. By Zsombor Egri

bottomedge height is less than the parent's height

1754. By Zsombor Egri

do not overshoot; API fix

1755. By Zsombor Egri

commit signals test

1756. By Zsombor Egri

collapse signals test

1757. By Zsombor Egri

collapse signals test

1758. By Zsombor Egri

fixing doc for contentUrl and contentComponent

1759. By Zsombor Egri

wording fix

1760. By Zsombor Egri

wording fix

1761. By Zsombor Egri

typo fix

1762. By Zsombor Egri

property fix

1763. By Zsombor Egri

qdoc tag fix

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Tim Peeters (tpeeters) wrote :

The hint text is not centered any more (in gallery), see https://www.dropbox.com/s/uvwzak5eumwcgnp/Screenshot%202015-11-26%2011.04.38.png?dl=0

Revision history for this message
Tim Peeters (tpeeters) wrote :

> > 304 - // we need to clip because the header does not have a background
> > 305 - clip: true
> > we shouldn't just remove this. It is no longer needed with the new PageHeader,
> > but apps may still > use the old AppHeader (by configuring Page.head, instead of
> > using Page.header).

> If we have this clip set, the bottom edge content header will always be clipped. I am open
> for solutions.

Removing it will break current apps. The solution for UITK 1.4/2.0 will come easy, because there we will remove the old AppHeader.

For now, we could add a solid background to the AppHeader that is the same color as the MainView.headerColor. However, if the app has a background texture or gradient (old designs), the header does not match any more, so it may mess up existing apps. An alternative is to force people to switch to the new header when they use APL.

1764. By Zsombor Egri

commit on collapse test case

1765. By Zsombor Egri

region operation tests

1766. By Zsombor Egri

alternate default region content

1767. By Zsombor Egri

rest order reshufled, covered tests removed

1768. By Zsombor Egri

dragEnded test

1769. By Zsombor Egri

end drag in region commits to teh region top

1770. By Zsombor Egri

collapse when the drag ends in an area that is not covered by any region

1771. By Zsombor Egri

remove tests covered by default check

1772. By Zsombor Egri

validate and warn on overlapping regions

1773. By Zsombor Egri

refining region validation

1774. By Zsombor Egri

auto-collapse added if content has PageHeader; remove warning about nesting page

1775. By Zsombor Egri

remove style specific test

1776. By Zsombor Egri

remove fill reset as it destroys the style overriden one, causing the nint to be mislocated

1777. By Zsombor Egri

change default region limits in gallery

1778. By Zsombor Egri

redo validation to validate runtime too

1779. By Zsombor Egri

remove comments, tweaking done meantime seemed to help

1780. By Zsombor Egri

staging sync

Revision history for this message
Zsombor Egri (zsombi) wrote :

> > > 304 - // we need to clip because the header does not have a background
> > > 305 - clip: true
> > > we shouldn't just remove this. It is no longer needed with the new
> PageHeader,
> > > but apps may still > use the old AppHeader (by configuring Page.head,
> instead of
> > > using Page.header).
>
> > If we have this clip set, the bottom edge content header will always be
> clipped. I am open
> > for solutions.
>
> Removing it will break current apps. The solution for UITK 1.4/2.0 will come
> easy, because there we will remove the old AppHeader.

No, we cannot. That's the problem. We announce the deprecation 1.3, which means we have to keep it one more version and then remove it. So as long as 1.4 will have the AppHeader, it will cause trouble, because people will still use it.

>
> For now, we could add a solid background to the AppHeader that is the same
> color as the MainView.headerColor. However, if the app has a background
> texture or gradient (old designs), the header does not match any more, so it
> may mess up existing apps. An alternative is to force people to switch to the
> new header when they use APL.

What if we do it in a separate MR? Like immediately after this...

Revision history for this message
Zsombor Egri (zsombi) wrote :

> The hint text is not centered any more (in gallery), see https://www.dropbox.c
> om/s/uvwzak5eumwcgnp/Screenshot%202015-11-26%2011.04.38.png?dl=0

Fixed, it was the anchor.fill introduced by the style overridden by the cpp resetFill() :/

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
1781. By Zsombor Egri

activeRegion changes optimized

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Tim Peeters (tpeeters) wrote :

> > > > 304 - // we need to clip because the header does not have a background
> > > > 305 - clip: true
> > > > we shouldn't just remove this. It is no longer needed with the new
> > PageHeader,
> > > > but apps may still > use the old AppHeader (by configuring Page.head,
> > instead of
> > > > using Page.header).
> >
> > > If we have this clip set, the bottom edge content header will always be
> > clipped. I am open
> > > for solutions.
> >
> > Removing it will break current apps. The solution for UITK 1.4/2.0 will come
> > easy, because there we will remove the old AppHeader.
>
> No, we cannot. That's the problem. We announce the deprecation 1.3, which
> means we have to keep it one more version and then remove it. So as long as
> 1.4 will have the AppHeader, it will cause trouble, because people will still
> use it.
>
> >
> > For now, we could add a solid background to the AppHeader that is the same
> > color as the MainView.headerColor. However, if the app has a background
> > texture or gradient (old designs), the header does not match any more, so it
> > may mess up existing apps. An alternative is to force people to switch to
> the
> > new header when they use APL.
>
> What if we do it in a separate MR? Like immediately after this...

If we do it *after*, then we have the header+APL broken until that next MR lands...

Revision history for this message
Tim Peeters (tpeeters) wrote :

looks good. But we MUST give the AppHeader a background in a separate MR before landing this in trunk.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)
Revision history for this message
Tim Peeters (tpeeters) wrote :

The desire to give the AppHeader a background as stated above, was reported in this bug: https://bugs.launchpad.net/ubuntu/+source/ubuntu-ui-toolkit/+bug/1531457

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