Code review comment for lp:~caldoria+inkscape.dev/inkscape/clonetiler-absolute-units

Revision history for this message
Mark (caldoria) wrote :

Hi,

Thanks for reviewing the code. I've tried to address your comments. There seem to be no conflicts at the moment.

As to what I did in the code, I changed the widgets in the clonetiler dialog to be able to deal with units. Before one could only input dimensionless quantities via that dialog. I have only changed the "Shift" and "Scale" pages, as these seem to me to be the only ones that would reasonably benefit from unit quantities. This mainly meant replacing the relevant input widgets with UI::Widget::ScalarUnit and adding corresponding UI::Widget::UnitMenu widgets. I've based my changes on the code for the Transformation dialog. I've added appropriate signal handling functions that make use of the existing model of storing all quantities within the preferences subsystem before calculating the required transforms. I also made some changes to the code actually calculating the transforms so that the units are respected.

I also made a small change to the "Labelled" widget. Before a label would be placed into the layout manager even if no label (empty string) is specified, which could yield an unsightly space. I've made this conditional on non-empty strings. I've done this because the ScalarUnit widget is a Labelled, and while I don't want to label the widget, I do want to make use of the unit capabilities.

Thanks,
Mark

« Back to merge proposal