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

🔧 Summary Card: Change the font size implementation to be simpler & fix import dashboard #3871

Merged
139 changes: 41 additions & 98 deletions packages/desktop-client/src/components/reports/SummaryNumber.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import { PrivacyFilter } from '../PrivacyFilter';
import { chartTheme } from './chart-theme';
import { LoadingIndicator } from './LoadingIndicator';

const FONT_SIZE_SCALE_FACTOR = 0.9;
const MAX_RECURSION_DEPTH = 10;
const FONT_SIZE_SCALE_FACTOR = 1.6;
const CONTAINER_MARGIN = 8;

type SummaryNumberProps = {
value: number;
Expand All @@ -32,61 +32,28 @@ export function SummaryNumber({
initialFontSize = 14,
fontSizeChanged,
}: SummaryNumberProps) {
const [fontSize, setFontSize] = useState<number>(0);
const [fontSize, setFontSize] = useState<number>(initialFontSize);
const refDiv = useRef<HTMLDivElement>(null);
const offScreenRef = useRef<HTMLDivElement>(null);
const displayAmount = amountToCurrency(Math.abs(value)) + suffix;

const adjustFontSizeBinary = (minFontSize: number, maxFontSize: number) => {
if (!offScreenRef.current || !refDiv.current) return;

const offScreenDiv = offScreenRef.current;
const refDivCurrent = refDiv.current;

const binarySearchFontSize = (
min: number,
max: number,
depth: number = 0,
) => {
if (depth >= MAX_RECURSION_DEPTH) {
setFontSize(min);
return;
}

const testFontSize = (min + max) / 2;
offScreenDiv.style.fontSize = `${testFontSize}px`;

requestAnimationFrame(() => {
const isOverflowing =
offScreenDiv.scrollWidth > refDivCurrent.clientWidth ||
offScreenDiv.scrollHeight > refDivCurrent.clientHeight;
const handleResize = debounce(() => {
if (!refDiv.current) return;

if (isOverflowing) {
binarySearchFontSize(min, testFontSize, depth + 1);
} else {
const isUnderflowing =
offScreenDiv.scrollWidth <=
refDivCurrent.clientWidth * FONT_SIZE_SCALE_FACTOR ||
offScreenDiv.scrollHeight <=
refDivCurrent.clientHeight * FONT_SIZE_SCALE_FACTOR;
const { clientWidth, clientHeight } = refDiv.current;
const width = clientWidth; // no margin required on left and right
const height = clientHeight - CONTAINER_MARGIN * 2; // account for margin top and bottom

if (isUnderflowing && testFontSize < max) {
binarySearchFontSize(testFontSize, max, depth + 1);
} else {
setFontSize(testFontSize);
if (initialFontSize !== testFontSize && fontSizeChanged) {
fontSizeChanged(testFontSize);
}
}
}
});
};
const calculatedFontSize = Math.min(
(width * FONT_SIZE_SCALE_FACTOR) / displayAmount.toString().length,
height, // Ensure the text fits vertically by using the height as the limiting factor
);
MikesGlitch marked this conversation as resolved.
Show resolved Hide resolved

binarySearchFontSize(minFontSize, maxFontSize);
};
setFontSize(calculatedFontSize);

const handleResize = debounce(() => {
adjustFontSizeBinary(14, 200);
}, 250);
if (calculatedFontSize !== initialFontSize && fontSizeChanged) {
fontSizeChanged(calculatedFontSize);
}
}, 100);

const ref = useResizeObserver(handleResize);
const mergedRef = useMergedRefs(ref, refDiv);
Expand All @@ -95,53 +62,29 @@ export function SummaryNumber({
<>
{loading && <LoadingIndicator />}
{!loading && (
<>
<div
ref={offScreenRef}
style={{
position: 'fixed',
left: '-999px',
top: '-999px',
fontSize: `${initialFontSize}px`,
lineHeight: 1,
visibility: 'hidden',
whiteSpace: 'nowrap',
padding: 8,
}}
>
<PrivacyFilter>
{amountToCurrency(Math.abs(value))}
{suffix}
</PrivacyFilter>
</div>

<View
ref={mergedRef as Ref<HTMLDivElement>}
role="text"
aria-label={`${value < 0 ? 'Negative' : 'Positive'} amount: ${amountToCurrency(Math.abs(value))}${suffix}`}
style={{
alignItems: 'center',
flexGrow: 1,
flexShrink: 1,
width: '100%',
height: '100%',
maxWidth: '100%',
fontSize: `${fontSize}px`,
lineHeight: 1,
padding: 8,
justifyContent: 'center',
transition: animate ? 'font-size 0.3s ease' : '',
color: value < 0 ? chartTheme.colors.red : chartTheme.colors.blue,
}}
>
<span aria-hidden="true">
<PrivacyFilter>
{amountToCurrency(Math.abs(value))}
{suffix}
</PrivacyFilter>
</span>
</View>
</>
<View
ref={mergedRef as Ref<HTMLDivElement>}
role="text"
aria-label={`${value < 0 ? 'Negative' : 'Positive'} amount: ${displayAmount}`}
style={{
alignItems: 'center',
flexGrow: 1,
flexShrink: 1,
width: '100%',
height: '100%',
maxWidth: '100%',
fontSize,
lineHeight: 1,
margin: `${CONTAINER_MARGIN}px 0`,
justifyContent: 'center',
transition: animate ? 'font-size 0.3s ease' : '',
color: value < 0 ? chartTheme.colors.red : chartTheme.colors.blue,
}}
>
<span aria-hidden="true">
<PrivacyFilter>{displayAmount}</PrivacyFilter>
</span>
</View>
)}
</>
);
Expand Down
1 change: 1 addition & 0 deletions packages/loot-core/src/server/dashboard/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ const exportModel = {
'spending-card',
'custom-report',
'markdown-card',
'summary-card',
].includes(widget.type)
) {
throw new ValidationError(
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/3871.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Maintenance
authors: [MikesGlitch]
---

Summary Report: Update font size implementation to be simpler