Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1521)

Issue 25460043: Simplify bundle vis centering

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 5 months ago by gary.poster
Modified:
10 years, 5 months ago
Reviewers:
mp+194932, matthew.scott
Visibility:
Public.

Description

Simplify bundle vis centering It turns out that using a polygon centroid isn't quite what our eyes expected, and also caused a bug when elements were away from the center mass of services. Using the bounding box gives more expected results. I didn't add a test, because I argued to myself that this was a visual thing, but I'm really lying to myself, and if you want to call me out on it, please feel free to appeal to my better self. :-) To QA, look at a lot of bundles, like those from "hatch", "jorge", and "gary". Then also look at the problematic one, http://localhost:8888/sidebar/search/bundle/~makyo/mediawiki-scalable/5/mediawiki-scalable/ . Compare it with the rendering in trunk (like on comingsoon) to see the difference. https://code.launchpad.net/~gary/juju-gui/bug1250547/+merge/194932 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : Simplify bundle vis centering #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -11 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M app/views/topology/bundle.js View 1 chunk +4 lines, -11 lines 0 comments Download

Messages

Total messages: 3
gary.poster
Please take a look.
10 years, 5 months ago (2013-11-12 20:02:38 UTC) #1
matthew.scott
LGTM! \o/
10 years, 5 months ago (2013-11-12 20:07:01 UTC) #2
gary.poster
10 years, 5 months ago (2013-11-12 20:26:07 UTC) #3
*** Submitted:

Simplify bundle vis centering

It turns out that using a polygon centroid isn't quite what our eyes expected,
and also caused a bug when elements were away from the center mass of services. 
Using the bounding box gives more expected results.

I didn't add a test, because I argued to myself that this was a visual thing,
but I'm really lying to myself, and if you want to call me out on it, please
feel free to appeal to my better self. :-)

To QA, look at a lot of bundles, like those from "hatch", "jorge", and "gary". 
Then also look at the problematic one,
http://localhost:8888/sidebar/search/bundle/~makyo/mediawiki-scalable/5/media...
.  Compare it with the rendering in trunk (like on comingsoon) to see the
difference.

R=matthew.scott
CC=
https://codereview.appspot.com/25460043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b