Skip to content

Commit

Permalink
chore: remove typography selection in app theming (#36110)
Browse files Browse the repository at this point in the history
/ok-to-test tags="@tag.All"

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

## Summary by CodeRabbit

- **New Features**
- Introduced new hooks for icon management: `useIconDensity` and
`useIconSizing`.

- **Improvements**
- Refactored hooks to enhance performance by using `useMemo` instead of
state management, leading to more efficient calculations for icon size,
density, spacing, and typography.
- Streamlined theme management by consolidating logic and removing
unnecessary properties, improving clarity and maintainability.
- Optimized CSS class name management within the `useCssTokens` hook for
better performance.

- **Bug Fixes**
- Removed `fontFamily` property from various components, which may
affect text rendering but simplifies font management.

- **Documentation**
- Updated comments and documentation to reflect changes in typography
and theme handling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10786030048>
> Commit: a0d1258
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10786030048&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Tue, 10 Sep 2024 06:00:30 UTC
<!-- end of auto-generated comment: Cypress test results  -->

---------

Co-authored-by: Pawan Kumar <[email protected]>
  • Loading branch information
jsartisan and Pawan Kumar authored Sep 13, 2024
1 parent 71b44a1 commit 049a8a9
Show file tree
Hide file tree
Showing 19 changed files with 129 additions and 396 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,7 @@ describe(
anvilSnapshot.setAccentColor("#0080ff");
});

it("3. Typography", () => {
anvilSnapshot.setTypography("Inter");

anvilSnapshot.matchSnapshotForCanvasMode("AppThemingTypography");
anvilSnapshot.matchSnapshotForPreviewMode("AppThemingTypography");
anvilSnapshot.matchSnapshotForDeployMode("AppThemingTypography");

anvilSnapshot.setTypography("System Default");
});

it("4. Density", () => {
it("3. Density", () => {
["Tight", "Regular", "Loose"].forEach((density) => {
anvilSnapshot.setDensity(density);

Expand All @@ -58,7 +48,7 @@ describe(
});
});

it("5. Sizing", () => {
it("4. Sizing", () => {
["Small", "Regular", "Big"].forEach((size) => {
anvilSnapshot.setSizing(size);

Expand All @@ -68,7 +58,7 @@ describe(
});
});

it("6. Corners", () => {
it("5. Corners", () => {
["0px", "6px", "20px"].forEach((corner) => {
anvilSnapshot.setCorners(corner);

Expand All @@ -78,7 +68,7 @@ describe(
});
});

it("7. Icon Style", () => {
it("6. Icon Style", () => {
["Filled", "Outlined"].forEach((iconStyle) => {
anvilSnapshot.setIconStyle(iconStyle);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ export * from "./useSpacing";
export * from "./useTypography";
export * from "./useTheme";
export * from "./useCssTokens";
export * from "./useIconDensity";
export * from "./useIconSizing";
Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
import { css } from "@emotion/css";
import { useEffect, useState } from "react";
import { cssRule, getTypographyClassName } from "../../utils";
import { useMemo } from "react";
import { objectKeys } from "@appsmith/utils";

import type { Theme } from "../../theme";
import type { FontFamily, ThemeToken, Typography } from "../../token";
import type { ThemeToken, Typography } from "../../token";
import { cssRule, getTypographyClassName } from "../../utils";

const fontFamilyCss = (fontFamily?: FontFamily) => {
const fontFamilyCss = () => {
const fontFamilyCss =
fontFamily && fontFamily !== "System Default"
? `${fontFamily}, sans-serif`
: "-apple-system, 'BlinkMacSystemFont', 'Segoe UI', 'Roboto', 'Ubuntu', sans-serif";
"-apple-system, 'BlinkMacSystemFont', 'Segoe UI', 'Roboto', 'Ubuntu', sans-serif";

return `font-family: ${fontFamilyCss}; --font-family: ${fontFamilyCss}`;
};

const getTypographyCss = (typography: Typography) => {
return css`
${Object.keys(typography).reduce((prev, key) => {
${objectKeys(typography).reduce((prev, key) => {
const currentKey = key as keyof Typography;
const { after, before, fontSize, lineHeight } = typography[currentKey];
return (
Expand Down Expand Up @@ -55,47 +54,39 @@ const getColorCss = (color: ThemeToken["color"]) => {
};

export function useCssTokens(props: Theme) {
const { color, colorMode, fontFamily, typography, ...restTokens } = props;
const { color, colorMode, typography, ...restTokens } = props;

const [colorClassName, setColorClassName] = useState<string>();
const [colorModeClassName, setColorModeClassName] = useState<string>();
const [fontFamilyClassName, setFontFamilyClassName] = useState<string>();
const [typographyClassName, setTypographyClassName] = useState<string>();
const [providerClassName, setProviderClassName] = useState<string>();

useEffect(() => {
const colorClassName = useMemo(() => {
if (color != null) {
setColorClassName(css`
return css`
${getColorCss(color)}
`);
`;
}
}, [color]);

useEffect(() => {
const typographyClassName = useMemo(() => {
if (typography != null) {
setTypographyClassName(css`
return css`
${getTypographyCss(typography)}
`);
`;
}
}, [typography]);

useEffect(() => {
setFontFamilyClassName(css`
${fontFamilyCss(fontFamily)}
`);
}, [fontFamily]);
const fontFamilyClassName = css`
${fontFamilyCss()}
`;

useEffect(() => {
setProviderClassName(css`
const providerClassName = useMemo(() => {
return css`
${cssRule(restTokens)};
`);
`;
}, [restTokens]);

useEffect(() => {
const colorModeClassName = useMemo(() => {
if (colorMode != null) {
setColorModeClassName(css`
return css`
color-scheme: ${colorMode};
`);
`;
}
}, [colorMode]);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,17 @@
import { useEffect, useState } from "react";
import type { IconDensity, TokenObj } from "../../token";
import { useMemo } from "react";
import type { IconDensity } from "../../token";

export const useIconDensity = (density: IconDensity, userDensity = 1) => {
const [strokeWidth, setStrokeWidth] = useState<TokenObj>();

useEffect(() => {
const strokeWidth = useMemo(() => {
switch (true) {
case userDensity < 1:
setStrokeWidth(density.tight);
break;
return density.tight;
case userDensity === 1:
setStrokeWidth(density.regular);
break;
return density.regular;
case userDensity > 1:
setStrokeWidth(density.loose);
break;
return density.loose;
default:
setStrokeWidth(density.regular);
break;
return density.regular;
}
}, [userDensity, density]);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,17 @@
import { useEffect, useState } from "react";
import type { IconSizing, TokenObj } from "../../token";
import { useMemo } from "react";
import type { IconSizing } from "../../token";

export const useIconSizing = (sizing: IconSizing, userSizing = 1) => {
const [iconSize, setIconSize] = useState<TokenObj>();

useEffect(() => {
const iconSize = useMemo(() => {
switch (true) {
case userSizing < 1:
setIconSize(sizing.small);
break;
return sizing.small;
case userSizing === 1:
setIconSize(sizing.regular);
break;
return sizing.regular;
case userSizing > 1:
setIconSize(sizing.big);
break;
return sizing.big;
default:
setIconSize(sizing.regular);
break;
return sizing.regular;
}
}, [userSizing, sizing]);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useState } from "react";
import { useMemo } from "react";
import { calculateScales } from "./calculateScales";

import type { TokenObj, TokenScaleConfig } from "../../token";
Expand All @@ -8,7 +8,7 @@ export const getSizing = (
userDensity = 1,
userSizing = 1,
count = 200,
) => {
): TokenObj => {
const { userDensityRatio = 1, userSizingRatio = 1, V, ...rest } = sizing;
const ratio = userDensity * userDensityRatio + userSizing * userSizingRatio;

Expand All @@ -35,11 +35,9 @@ export const useSizing = (
userDensity = 1,
userSizing = 1,
) => {
const [sizing, setSizing] = useState<TokenObj>();

useEffect(() => {
setSizing(getSizing(config, userDensity, userSizing));
}, [userDensity, userSizing, config]);
const sizing = useMemo(() => {
return getSizing(config, userDensity, userSizing);
}, [config, userDensity, userSizing]);

return {
sizing,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useEffect, useState } from "react";
import { useMemo } from "react";
import { calculateScales } from "./calculateScales";

import type { TokenObj, TokenScaleConfig } from "../../token";
import type { TokenScaleConfig } from "../../token";

export const getSpacing = (
spacing: TokenScaleConfig,
Expand Down Expand Up @@ -36,16 +36,13 @@ export const useSpacing = (
userDensity = 1,
userSizing = 1,
) => {
const [outerSpacing, setOuterSpacing] = useState<TokenObj>();
const [innerSpacing, setInnerSpacing] = useState<TokenObj>();
const outerSpacing = useMemo(() => {
return getSpacing(outerSpacingConfig, userDensity, userSizing);
}, [outerSpacingConfig, userDensity, userSizing]);

useEffect(() => {
setOuterSpacing(getSpacing(outerSpacingConfig, userDensity, userSizing));
}, [userDensity, userSizing, outerSpacingConfig]);

useEffect(() => {
setInnerSpacing(getSpacing(innerSpacingConfig, userDensity, userSizing));
}, [userDensity, userSizing, innerSpacingConfig]);
const innerSpacing = useMemo(() => {
return getSpacing(innerSpacingConfig, userDensity, userSizing);
}, [innerSpacingConfig, userDensity, userSizing]);

return {
outerSpacing,
Expand Down
Loading

0 comments on commit 049a8a9

Please sign in to comment.