Closed GoogleCodeExporter closed 9 years ago
Original comment by codedr...@gmail.com
on 2 Jul 2009 at 5:40
proposed icons:
http://tango.freedesktop.org/static/cvs/tango-art-libre/22x22/actions/align-
horizontal-left.png
http://tango.freedesktop.org/static/cvs/tango-art-libre/22x22/actions/align-
horizontal-right.png
http://tango.freedesktop.org/static/cvs/tango-art-libre/22x22/actions/align-
horizontal-center.png
http://tango.freedesktop.org/static/cvs/tango-art-libre/22x22/actions/align-vert
ical-
bottom.png
http://tango.freedesktop.org/static/cvs/tango-art-libre/22x22/actions/align-vert
ical-
center.png
http://tango.freedesktop.org/static/cvs/tango-art-libre/22x22/actions/align-vert
ical-
top.png
Original comment by rusn...@gmail.com
on 14 Aug 2009 at 9:06
I just commited the preliminary Align feature in r397 r398 r399. What does
preliminary mean?
a) only circle, ellipse, rect and line are supported at the moment
b) bounding boxes of elements do not move
c) code is too long - e.g parseInt(elem.getAttribute('r')) could be turned into
getAttrInt(elem, 'r') or something similar
Original comment by rusn...@gmail.com
on 17 Aug 2009 at 7:08
I just noticed that I added buttons for align functions, but they are already
present in a hidden flyout. This should be fixed as well.
Original comment by rusn...@gmail.com
on 17 Aug 2009 at 7:11
Pavol, cool stuff! :)
Your work definitely uncovered why this is a trickier feature than I thought.
Anyway, here's my code review:
> var minx = 100000, maxx = -100000, miny = 100000, maxy = -100000;
Please use Number.MIN_VALUE and Number.MAX_VALUE.
> case 'circle':
> if (parseInt(elem.getAttribute(('cx'))) -
parseInt(elem.getAttribute(('r'))) <
minx) minx = parseInt(elem.getAttribute(('cx'))) -
parseInt(elem.getAttribute(('r')));
There is a getBBox() function on the SvgCanvas that gives the bounding box of a
given
element. It seems you could use this for all element types and then just store
the
dxs and dys that each element has to move.
> case 'l': // left (horizontal)
> elem.setAttribute('cx', minx+parseInt(elem.getAttribute(('r'))));
> break;
Your code at the moment is also not undo-able. Perhaps you could modify
moveSelectedElements() so that it can accept either an array or a single number
for
dx,dy and then use that to move the elements? This would significantly reduce
the
code of this feature to just figuring out how much (dx,dy) to move each element
and
then just calling moveSelectedElements (which handles moving, storing the
undo-able
batch command and updating the selector boxes).
Great work so far!
Original comment by codedr...@gmail.com
on 17 Aug 2009 at 12:09
Btw, the getBBox() function also returns the bbox that corresponds to rotated
elements too (I'm not sure but I think your existing code doesn't handle
rotations yet).
Original comment by codedr...@gmail.com
on 17 Aug 2009 at 12:11
We don't necessarily need the flyouts since there are so few tools in
multi-select
panel right now. We can do that in a future release.
Original comment by codedr...@gmail.com
on 17 Aug 2009 at 12:25
Original comment by codedr...@gmail.com
on 19 Aug 2009 at 2:31
Original comment by codedr...@gmail.com
on 20 Aug 2009 at 9:01
Align is now fully working. There are some problems with rotated elements
though and
this needs more testing
Original comment by rusn...@gmail.com
on 23 Aug 2009 at 5:17
Fixed aligning rotated elements in r455.
Original comment by codedr...@gmail.com
on 24 Aug 2009 at 3:30
Original issue reported on code.google.com by
codedr...@gmail.com
on 2 Jul 2009 at 5:40