D18476: Fixing

[Alexander Semke]

asemke added inline comments.

INLINE COMMENTS

> Axis.cpp:1569
>  	td.setDefaultFont(labelsFont);
> -
> +	double cosinus = cos(labelsRotationAngle * M_PI / 180);
> +	double sinus = sin(labelsRotationAngle * M_PI / 180);

cosinus and sinus only used in the definition of diffx and diffy and can be used there directly.

> Axis.cpp:1572
>  	for ( int i = 0; i < majorTickPoints.size(); i++ ) {
> -		if (labelsFormat == Axis::FormatDecimal || labelsFormat == Axis::FormatScientificE) {
> +		if ((orientation == Axis::AxisHorizontal && plot->xRangeFormat() == CartesianPlot::Numeric) ||
> +				(orientation == Axis::AxisVertical && plot->yRangeFormat() == CartesianPlot::Numeric)) {

why do you need to check orientation here?

> Axis.cpp:1599
> +
> +			// move so, that the corner is at the same position than the tick when the rotation angle is not zero
> +			if (labelsRotationAngle >= 0.01) {

improve the wording a bit. Maybe something like

"for rotated labels (angle is not zero), align label's corner at the position of the tick"

> Axis.cpp:1600
> +			// move so, that the corner is at the same position than the tick when the rotation angle is not zero
> +			if (labelsRotationAngle >= 0.01) {
> +				if (labelsPosition == Axis::LabelsOut) {

if I set Rotation=90° and Offset=0pt, I'd expect the label to be positioned right next to the tick and aligned at the center of the rotated label. This is not the case with this logic.

> AxisDock.cpp:72
> +
> +	int rowCount = layout->rowCount();
> +	int columnCount = layout->columnCount();

let's use hard coded row numbers here instead of this for loop with all the logic that is not so easy to grasp.

> AxisDock.cpp:849
> +	const auto* plot = dynamic_cast<const CartesianPlot*>(m_axis->parentAspect());
> +	if (plot) {
> +		bool numeric = ( (m_axis->orientation() == Axis::AxisHorizontal && plot->xRangeFormat() == CartesianPlot::Numeric)

this check is not required. An axis has always a plot as the parent.

> AxisDock.cpp:911
> +	const auto* plot = dynamic_cast<const CartesianPlot*>(m_axis->parentAspect());
> +	if (plot) {
> +		bool numeric = ( (m_axis->orientation() == Axis::AxisHorizontal && plot->xRangeFormat() == CartesianPlot::Numeric)

same as above.

> AxisDock.cpp:919
> +			value = ui.sbMajorTicksIncrementNumeric->value();
> +		else {
> +			value = dtsbMajorTicksIncrement->value();

no brackets for one-liners.

> AxisDock.cpp:931
> +				ui.sbMajorTicksIncrementNumeric->setValue(value);
> +			else {
> +				dtsbMajorTicksIncrement->setValue(value);

no brackets for one-liners.

> AxisDock.cpp:1874
>  
> -	m_initializing = true;
> +	m_initializing = false;
>  	GuiTools::updatePenStyles(ui.cbLineStyle, ui.kcbLineColor->color());

why this change?

> AxisDock.cpp:1882
>  	GuiTools::updatePenStyles(ui.cbMinorGridStyle, ui.kcbMinorGridColor->color());
> -	m_initializing = false;
> +	m_initializing = true;
> +}

why this change?

REVISION DETAIL
  https://phabricator.kde.org/D18476

To: Murmele, asemke
Cc: sgerlach, yurchor, kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190124/01a1f92b/attachment-0001.html>