Connector improvements #6
Conversation
There was a problem hiding this comment.
Lots of good features here, but several things to tidy up.
General comments:
- avoid over-short variable names, and try to be consistent with how a similar thing has been named elsewhere in the codebase
- comments on functions if it's not obvious what they do
- the file probably needs splitting up - the point-along-curve logic is a clear candidate for extracting. If there's a bunch of common code needed, extract a new module directory.
src/geometry/types.rs
Outdated
| @@ -193,6 +194,7 @@ impl FromStr for LocSpec { | |||
| "r" => Ok(Self::RightEdge(len)), | |||
| "b" => Ok(Self::BottomEdge(len)), | |||
| "l" => Ok(Self::LeftEdge(len)), | |||
| "" => Ok(Self::PureLength(len)), | |||
There was a problem hiding this comment.
Presumably this means you have to do "#id@:50%" etc, which isn't very nice, although it does avoid the potential issue that ':' can (in theory) be part of element id values...
I'd originally envisioned #id:50%, but in theory (at least for absolute lengths without '%') something like #id:2 is a valid element id. Maybe #id@2, #id@50% would be doable?
…tidied up connector a bit
megamaths
force-pushed
the
connector_improvements
branch
from
b29f9ef to
5d68752
Compare
megamaths
force-pushed
the
connector_improvements
branch
from
5d68752 to
0f954d4
Compare
src/elements/line_offset.rs
Outdated
| } | ||
|
|
||
| fn get_point_along_polyline(el: &SvgElement, length: Length) -> Result<(f32, f32)> { | ||
| let (mut is_percent, mut dist) = match length { |
There was a problem hiding this comment.
Need to think about whether this should support the current 'negative value implies backward from the end' behaviour which Length::calc_offset() does - especially if that is used in <line> elements. Though maybe that needs revisiting anyway, as absolute values beyond either end are useful (e.g. for placing text) and the 'negative = backwards' logic would require knowing the length of the (poly)line in advance (e.g. 2 before start on a 50 unit line = '-52'), which is a bit silly. Anyway, could just reverse the points if implementing this does make sense, but probably defer for now and leave with current (clamping) behaviour.
Various connector improvements, can now do connectors with more than 2 corners, can refer to points on a line like element by using
#id@:40%and can make connector corners rounded.