Ubuntu UI Toolkit

Merge lp://staging/~nick-dedekind/ubuntu-ui-toolkit/round.floor.ceil-GridUnits into lp://staging/ubuntu-ui-toolkit

  1. round.floor.ceil-GridUnits
  2. Merge into trunk
Proposed by Nick Dedekind
Status: Rejected
Rejected by: Tim Peeters
Proposed branch: lp://staging/~nick-dedekind/ubuntu-ui-toolkit/round.floor.ceil-GridUnits
Merge into: lp://staging/ubuntu-ui-toolkit
Diff against target: 217 lines (+174/-0)
4 files modified
components.api (+21/-0)
modules/Ubuntu/Components/plugin/ucunits.cpp (+51/-0)
modules/Ubuntu/Components/plugin/ucunits.h (+5/-0)
tests/unit/tst_units/tst_units.cpp (+97/-0)
To merge this branch: bzr merge lp://staging/~nick-dedekind/ubuntu-ui-toolkit/round.floor.ceil-GridUnits
Reviewer Review Type Date Requested Status
Florian Boucault (community) Needs Fixing
PS Jenkins bot continuous-integration Approve
Review via email: mp+195796@code.staging.launchpad.net

Commit message

Added [floor/ceil/round]GridUnit function to get closest gu boundary position.

Description of the change

Added [floor/ceil/round]GridUnit function to get closest gu boundary position.

This is to enable us to easily align graphical items to the gu grid.
Use case:
Indicator panel items can be variable width (fixed height with aspect calculated width + variable length label) but should be aligned to the gu grid.

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

Why do we need this? What is the use case? What is the problem that we are trying to solve?

review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:840
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/1249/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/866
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/854
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/197
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/197
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/197/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/777
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/866
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/866/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/854
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/854/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3454
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/1543

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/1249/rebuild

review: Approve (continuous-integration)
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

> Why do we need this? What is the use case? What is the problem that we are
> trying to solve?

This is to enable us to easily align graphical items to the gu grid.
Use case:
Indicator panel items can be variable width (fixed height with aspect calculated width + variable length label) but should be aligned to the gu grid.

Currently there is a function in the indicators to calculate it, but should be in a common place so we can use it elsewhere. Units seemed the logical place to put it.

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

> > Why do we need this? What is the use case? What is the problem that we are
> > trying to solve?
>
> This is to enable us to easily align graphical items to the gu grid.
> Use case:
> Indicator panel items can be variable width (fixed height with aspect
> calculated width + variable length label) but should be aligned to the gu
> grid.
>
> Currently there is a function in the indicators to calculate it, but should be
> in a common place so we can use it elsewhere. Units seemed the logical place
> to put it.

Thanks for the explanation. Sorry for taking so long. Do you know where in the indicators code this is happening?

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

Found it! :)

Panel/Indicators/DefaultIndicatorWidget.qml

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

1) After reading function guRoundUp(width) in Panel/Indicators/DefaultIndicatorWidget.qml I did not expect to see a second 'multiple' argument in floorGridUnit, ceilGridUnit and roundGridUnit. I don't think they should have it.

2) I understand the use case for ceilGridUnit but not for floorGridUnit and roundGridUnit. Generally speaking, we avoid introducing APIs until we have a concrete use case for them. One of the main reason is that each API we have to support for a number of years.

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

Was this MR abolished?

If not, please resubmit it to our staging and ask for another review.

Unmerged revisions

840. By Nick Dedekind

Added [floor/ceil/round]GridUnit for finding gu boundaries.

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

to status/vote changes: