Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve image graph axis display in email reports #20953

Merged
merged 6 commits into from
Jul 9, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugins/ImageGraph/API.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public function get(
}

$useUnicodeFont = array(
'am', 'ar', 'el', 'fa', 'fi', 'he', 'ja', 'ka', 'ko', 'te', 'th', 'zh-cn', 'zh-tw',
'am', 'ar', 'el', 'fa', 'fi', 'fr', 'he', 'ja', 'ka', 'ko', 'te', 'th', 'zh-cn', 'zh-tw',
bx80 marked this conversation as resolved.
Show resolved Hide resolved
);
$languageLoaded = StaticContainer::get('Piwik\Translation\Translator')->getCurrentLanguage();
$font = self::getFontPath(self::DEFAULT_FONT);
Expand Down
22 changes: 20 additions & 2 deletions plugins/ImageGraph/StaticGraph.php
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,12 @@ protected function initpData()
}
}

$this->pData->setAxisDisplay(0, AXIS_FORMAT_CUSTOM, '\\Piwik\\Plugins\\ImageGraph\\formatYAxis');
// Use a different formating method if not using unifont
$formatMethodName = 'formatYAxis';
if (strpos($this->font, 'unifont') === false) {
$formatMethodName = 'formatYAxisNonUnifont';
}
$this->pData->setAxisDisplay(0, AXIS_FORMAT_CUSTOM, '\\Piwik\\Plugins\\ImageGraph\\' . $formatMethodName);

$this->pData->addPoints($this->abscissaSeries, self::ABSCISSA_SERIE_NAME);
$this->pData->setAbscissa(self::ABSCISSA_SERIE_NAME);
Expand Down Expand Up @@ -376,7 +381,7 @@ private static function hex2rgb($hexColor)
}

/**
* Global format method
* Global format method - unifont
*
* required to format y axis values using CpChart internal format callbacks
* @param $value
Expand All @@ -386,3 +391,16 @@ function formatYAxis($value)
{
return NumberFormatter::getInstance()->format($value);
}

/**
* Global format method - non-unifont
*
* required to format y axis values using CpChart internal format callbacks
* @param $value
* @return mixed
*/
function formatYAxisNonUnifont($value)
{
// Replace any narrow non-breaking spaces with non-breaking spaces as some fonts may not support it
return str_replace("\xE2\x80\xAF", "\xC2\xA0", NumberFormatter::getInstance()->format($value));
}
12 changes: 6 additions & 6 deletions plugins/ImageGraph/StaticGraph/GridGraph.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,18 @@ abstract class GridGraph extends StaticGraph

const DEFAULT_TICK_ALPHA = 20;
const DEFAULT_SERIE_WEIGHT = 0.5;
const LEFT_GRID_MARGIN = 4;
const LEFT_GRID_MARGIN = 20;
const BOTTOM_GRID_MARGIN = 10;
const TOP_GRID_MARGIN_HORIZONTAL_GRAPH = 1;
const RIGHT_GRID_MARGIN_HORIZONTAL_GRAPH = 4;
const TOP_GRID_MARGIN_HORIZONTAL_GRAPH = 10;
const RIGHT_GRID_MARGIN_HORIZONTAL_GRAPH = 20;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the changed screenshots it looks like the adjustments were actually only needed for the evolution chart.
The other charts now have additional unused space. Wouldn't it been have possible to only adjust the evolution one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other charts are also drawn very close to the edge of the image, for example the horizontal bar chart would likely clip the bar totals for numbers with four digits. I think adding few extra pixels of padding around them reduces the likelihood of other clipping issues and will look better if the images are embedded with other text.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we would actually need to automatically increase those margins if higher values are meant to be printed.
It might make sense to adjust the UI tests so they use higher values. I think we could achieve that without tracking additional numbers but simply modifying the results. I'll try something an maybe push some changes here to achieve that if easily possible.

const OUTER_TICK_WIDTH = 5;
const INNER_TICK_WIDTH = 0;
const LABEL_SPACE_VERTICAL_GRAPH = 7;

const HORIZONTAL_LEGEND_TOP_MARGIN = 5;
const HORIZONTAL_LEGEND_LEFT_MARGIN = 10;
const HORIZONTAL_LEGEND_BOTTOM_MARGIN = 10;
const VERTICAL_LEGEND_TOP_MARGIN = 8;
const VERTICAL_LEGEND_TOP_MARGIN = 10;
const VERTICAL_LEGEND_LEFT_MARGIN = 6;
const VERTICAL_LEGEND_MAX_WIDTH_PCT = 0.70;
const LEGEND_LINE_BULLET_WIDTH = 14;
Expand Down Expand Up @@ -358,7 +358,7 @@ protected function getGridTopMargin($horizontalGraph, $verticalLegend)
if ($horizontalGraph) {
$topMargin = $ordinateMaxHeight + self::TOP_GRID_MARGIN_HORIZONTAL_GRAPH + self::OUTER_TICK_WIDTH;
} else {
$topMargin = $ordinateMaxHeight / 2;
$topMargin = $ordinateMaxHeight / 2 + self::TOP_GRID_MARGIN_HORIZONTAL_GRAPH;
}

if ($this->showLegend && !$verticalLegend) {
Expand Down Expand Up @@ -398,7 +398,7 @@ protected function getGridRightMargin($horizontalGraph)
list($ordinateMaxWidth, $ordinateMaxHeight) = $this->getMaximumTextWidthHeight($this->ordinateSeries);
return self::RIGHT_GRID_MARGIN_HORIZONTAL_GRAPH + $ordinateMaxWidth;
} else {
return 0;
return self::RIGHT_GRID_MARGIN_HORIZONTAL_GRAPH;
}
}

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading