geido commented on code in PR #33099:
URL: https://github.com/apache/superset/pull/33099#discussion_r2056153813
##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx:
##########
@@ -92,6 +95,38 @@ class BigNumberVis extends PureComponent<BigNumberVizProps> {
);
}
+ renderMetricName(maxHeight: number) {
+ const { metricName, width, showMetricName } = this.props;
+ if (!showMetricName || !metricName) return null;
+
+ const text = metricName;
+
+ const container = this.createTemporaryContainer();
+ document.body.append(container);
+ const fontSize = computeMaxFontSize({
+ text,
+ maxWidth: width,
+ maxHeight,
+ className: 'metric-name',
+ container,
+ });
+ container.remove();
+
+ if (!showMetricName || !metricName) return null;
Review Comment:
Isn't this the same as the check above?
##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/utils.ts:
##########
@@ -40,20 +49,28 @@ const getFontSizeMapping = (
return acc;
}, {});
+const metricNameFontSizesMapping = getFontSizeMapping(
+ metricNameProportionValues,
+ sharedFontSizes,
+);
const headerFontSizesMapping = getFontSizeMapping(
headerProportionValues,
headerFontSizes,
);
const comparisonFontSizesMapping = getFontSizeMapping(
subheaderProportionValues,
- comparisonFontSizes,
+ sharedFontSizes,
);
+export const getMetricNameFontSize = (proportionValue: number) =>
+ metricNameFontSizesMapping[proportionValue] ??
+ sharedFontSizes[sharedFontSizes.length - 1];
Review Comment:
Is this logic correct here of always defaulting to the bigger font size?
##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/sharedControls.ts:
##########
@@ -21,106 +21,68 @@
import { t } from '@superset-ui/core';
import { CustomControlItem } from '@superset-ui/chart-controls';
-export const headerFontSize: CustomControlItem = {
- name: 'header_font_size',
- config: {
- type: 'SelectControl',
- label: t('Big Number Font Size'),
- renderTrigger: true,
- clearable: false,
- default: 0.4,
- // Values represent the percentage of space a header should take
- options: [
- {
- label: t('Tiny'),
- value: 0.2,
- },
- {
- label: t('Small'),
- value: 0.3,
- },
- {
- label: t('Normal'),
- value: 0.4,
- },
- {
- label: t('Large'),
- value: 0.5,
- },
- {
- label: t('Huge'),
- value: 0.6,
- },
- ],
- },
-};
+const FONT_SIZE_OPTIONS_SMALL = [
+ { label: t('Tiny'), value: 0.125 },
+ { label: t('Small'), value: 0.15 },
+ { label: t('Normal'), value: 0.175 },
+ { label: t('Large'), value: 0.185 },
+ { label: t('Huge'), value: 0.2 },
+];
-export const subtitleFontSize: CustomControlItem = {
- name: 'subtitle_font_size',
- config: {
- type: 'SelectControl',
- label: t('Subtitle Font Size'),
- renderTrigger: true,
- clearable: false,
- default: 0.15,
- // Values represent the percentage of space a subtitle should take
- options: [
- {
- label: t('Tiny'),
- value: 0.125,
- },
- {
- label: t('Small'),
- value: 0.15,
- },
- {
- label: t('Normal'),
- value: 0.2,
- },
- {
- label: t('Large'),
- value: 0.3,
- },
- {
- label: t('Huge'),
- value: 0.4,
- },
- ],
- },
-};
-export const subheaderFontSize: CustomControlItem = {
- name: 'subheader_font_size',
- config: {
- type: 'SelectControl',
- label: t('Subheader Font Size'),
- renderTrigger: true,
- clearable: false,
- default: 0.15,
- // Values represent the percentage of space a subheader should take
- options: [
- {
- label: t('Tiny'),
- value: 0.125,
- },
- {
- label: t('Small'),
- value: 0.15,
- },
- {
- label: t('Normal'),
- value: 0.2,
- },
- {
- label: t('Large'),
- value: 0.3,
- },
- {
- label: t('Huge'),
- value: 0.4,
- },
- ],
- },
-};
+const FONT_SIZE_OPTIONS_LARGE = [
+ { label: t('Tiny'), value: 0.2 },
+ { label: t('Small'), value: 0.25 },
+ { label: t('Normal'), value: 0.3 },
+ { label: t('Large'), value: 0.35 },
+ { label: t('Huge'), value: 0.4 },
+];
+
+function makeFontSizeControl(
+ name: string,
+ label: string,
+ defaultValue: number,
+ options: { label: string; value: number }[],
+): CustomControlItem {
+ return {
+ name,
+ config: {
+ type: 'SelectControl',
+ label: t(label),
+ renderTrigger: true,
+ clearable: false,
+ default: defaultValue,
+ options,
+ },
+ };
+}
+
+export const headerFontSize = makeFontSizeControl(
+ 'header_font_size',
+ 'Big Number Font Size',
+ 0.3,
Review Comment:
The default originally was` 0.4`. Is this change wanted?
##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx:
##########
@@ -92,6 +95,38 @@ class BigNumberVis extends PureComponent<BigNumberVizProps> {
);
}
+ renderMetricName(maxHeight: number) {
+ const { metricName, width, showMetricName } = this.props;
+ if (!showMetricName || !metricName) return null;
+
+ const text = metricName;
+
+ const container = this.createTemporaryContainer();
+ document.body.append(container);
+ const fontSize = computeMaxFontSize({
+ text,
+ maxWidth: width,
+ maxHeight,
+ className: 'metric-name',
+ container,
+ });
+ container.remove();
+
+ if (!showMetricName || !metricName) return null;
Review Comment:
Isn't this the same as the check above?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
