[Merge] lp:~artmello/camera-app/camera-app-gridlines into lp:camera-app
Florian Boucault
florian.boucault at canonical.com
Fri Nov 28 09:58:53 UTC 2014
Review: Needs Fixing
- making the grid of options 4 columns does not really solve the problem of too many options long term (especially with your other MR coming in); thankfully this was solved already in another branch's commit. I think we should merge the specific commit that fixes it: commit 428 from lp:~fboucault/camera-app/jpeg_quality
Merge command: bzr merge -c428 lp:~fboucault/camera-app/jpeg_quality
- icon "view-grid-symbolic" is not good enough. I asked Matthieu (tiheum) from design to provide a better one.
- visually the grid should not have lines around the picture, only the 4 lines inside it; these lines outside are not useful and they create a boxing feel to the UI
- no need to use a Binding object (I understand the consistency reason, but it won't work out for other settings)
- better to not instantiate the Rectangles at all when the grid is disabled; "Repeater.model: settings.gridEnabled ? gridlines.columns * gridlines.rows : 0" will do the trick
- using Rectangle.border to draw is slightly more expensive than using Rectangle.color
--
https://code.launchpad.net/~artmello/camera-app/camera-app-gridlines/+merge/243092
Your team Ubuntu Phablet Team is subscribed to branch lp:camera-app.
More information about the Ubuntu-reviews
mailing list