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

tweak(tupaiaWeb): RN-249: getting x-Name in viz export #5886

Merged
merged 7 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ interface VerticalTickProps {
export const VerticalTick = ({ x, y, payload }: VerticalTickProps) => {
const stringVal =
payload.value !== undefined && payload.value !== null ? String(payload.value) : '';
const marginX = 10;
const marginY = 5;

return (
<g transform={`translate(${x - 5},${y + 3})`}>
<g transform={`translate(${x + marginX},${y - marginY})`}>
<text
fontSize="13px"
transform="rotate(305)"
Expand Down
12 changes: 7 additions & 5 deletions packages/ui-chart-components/src/components/Axes/XAxis.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Tupaia
* Copyright (c) 2017 - 2023 Beyond Essential Systems Pty Ltd
* Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd
*/

import React from 'react';
Expand Down Expand Up @@ -41,13 +41,13 @@ const renderXAxisLabel = (
label: string | undefined,
fillColor: string | undefined,
isEnlarged: boolean,
isExporting: boolean,
isExporting: boolean
): LabelProps | undefined => {
if (label && isEnlarged && !isExporting) {
if (label && isEnlarged) {
return {
value: label,
fill: fillColor,
offset: -5,
offset: isExporting ? -50 : -5,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is intended to prevent overlapping between tick labels and the x-axis. Given the tick labels are restricted to a maximum number of characters (using ellipses for longer text) this approach should resolve the issue.

position: 'insideBottom',
};
}
Expand Down Expand Up @@ -76,6 +76,7 @@ interface XAxisProps {

export const XAxis = ({ config, report, isExporting = false, isEnlarged = false }: XAxisProps) => {
const fillColor = isExporting ? DARK_BLUE : getContrastTextColor();
const tickMargin = isExporting ? 20 : 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the space between the x-axis label and tick label.

const { Bar, Composed } = ChartType;
const { chartType, chartConfig } = config;
const { data = [] } = report;
Expand Down Expand Up @@ -154,6 +155,7 @@ export const XAxis = ({ config, report, isExporting = false, isEnlarged = false
return { left: 0, right: 10 };
};


const renderVerticalTick = (tickProps: TickProps & { x: number; y: number }) => {
const { payload, x, y } = tickProps;

Expand All @@ -177,7 +179,7 @@ export const XAxis = ({ config, report, isExporting = false, isEnlarged = false
tick={getXAxisTickMethod()}
tickFormatter={formatXAxisTick}
padding={getXAxisPadding()}
tickSize={6}
tickMargin={tickMargin}
{...(isTimeSeries ? AXIS_TIME_PROPS : { dataKey: 'name' })}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ const getRealDataKeys = (chartConfig: CartesianChartConfig['chartConfig'] | {})
const getLegendAlignment = (
legendPosition: LegendPosition,
isExporting: boolean,
): Pick<LegendProps, 'align' | 'verticalAlign' | 'layout'> => {
): Pick<LegendProps, 'align' | 'verticalAlign'> => {
if (isExporting) {
return { verticalAlign: 'top', align: 'right', layout: 'vertical' };
return { verticalAlign: 'top', align: 'right' };
}
if (legendPosition === 'bottom') {
return { verticalAlign: 'bottom', align: 'center' };
Expand Down Expand Up @@ -218,7 +218,7 @@ export const CartesianChart = ({
const aspect = !isEnlarged && !isMobileSize && !isExporting ? 1.6 : undefined;
const height = getHeight(isExporting, isEnlarged, hasLegend, isMobileSize);

const { verticalAlign, align, layout } = getLegendAlignment(legendPosition, isExporting);
const { verticalAlign, align } = getLegendAlignment(legendPosition, isExporting);

const presentationOptions = 'presentationOptions' in config ? config.presentationOptions : {};

Expand Down Expand Up @@ -252,7 +252,6 @@ export const CartesianChart = ({
<Legend
verticalAlign={verticalAlign}
align={align}
layout={layout}
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 'layout' property (whether 'horizontal' or 'vertical') does not affect the custom component as expected. This is because the component already has its own styling, including alignment and flex properties. Therefore, the layout property is not needed in this case.

content={getCartesianLegend({
chartConfig,
getIsActiveKey,
Expand Down
20 changes: 7 additions & 13 deletions packages/ui-chart-components/src/components/Reference/Legend.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,13 @@ import { isMobile } from '../../utils';

const LegendContainer = styled.div<{
$position?: LegendPosition;
$isExporting?: boolean;
}>`
display: flex;
flex-wrap: ${props => (props.$isExporting ? 'nowrap' : 'wrap')};
justify-content: ${props => (props.$position === 'bottom' ? 'center' : 'flex-start')};
flex-direction: ${props => (props.$isExporting ? 'column' : 'row')};
flex-wrap: wrap;
justify-content: center;
flex-direction: row;
// Add more padding at the bottom for exports
padding: ${props => {
if (props.$isExporting) {
return '1em';
}
return props.$position === 'bottom' ? '1rem 0 0 3.5rem' : '0 0 2rem 0';
}};
padding: ${props => (props.$position === 'bottom' ? '1em 0 0 3.5em' : '0 0 3em 0')};
`;

const PieLegendContainer = styled(LegendContainer)`
Expand Down Expand Up @@ -55,7 +49,7 @@ const LegendItem = styled(({ isExporting, ...props }) => <MuiButton {...props} /
.MuiButton-label {
display: flex;
white-space: nowrap;
align-items: ${isExporting => (isExporting ? 'left' : 'center')};
align-items: center;
color: ${({ theme, isExporting }) => getLegendTextColor(theme, isExporting)};
> div {
width: ${isExporting => (isExporting ? '100%' : '')};
Expand Down Expand Up @@ -129,7 +123,7 @@ export const getPieLegend =
({ payload }: any) => {
const isMobileSize = isMobile(isExporting);
return (
<PieLegendContainer $position={legendPosition} $isExporting={isExporting}>
<PieLegendContainer $position={isExporting ? 'top' : legendPosition}>
{payload.map(({ color, value, payload: item }: TooltipPayload) => {
const displayValue = getPieLegendDisplayValue(
value,
Expand Down Expand Up @@ -173,7 +167,7 @@ export const getCartesianLegend =
({ payload }: any) => {
const isMobileSize = isMobile(isExporting);
return (
<LegendContainer $position={legendPosition} $isExporting={isExporting}>
<LegendContainer $position={isExporting ? 'top' : legendPosition}>
{payload.map(({ color, value, dataKey }: TooltipPayload) => {
// check the type here because according to TooltipPayload, value can be a number or a readonly string | number array
const displayValue = (typeof value === 'string' && chartConfig?.[value]?.label) || value;
Expand Down