From 7469f58709e69fba76550cbda9adbd119d7a8921 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Fern=C3=A1ndez?= Date: Tue, 1 Aug 2023 12:44:25 +0200 Subject: [PATCH] GLDS: Check if tokens are used as borderRadius values (#71187) --- .eslintrc | 1 + packages/grafana-eslint-rules/README.md | 6 ++ packages/grafana-eslint-rules/index.cjs | 2 + .../rules/no-border-radius-literal.cjs | 61 +++++++++++++++++++ .../DateTimePickers/DatePicker/DatePicker.tsx | 3 +- .../TimeRangePicker/TimePickerCalendar.tsx | 3 +- .../TimeRangePicker/TimePickerContent.tsx | 2 +- .../TimeZonePicker/TimeZoneOffset.tsx | 2 +- .../components/FileDropzone/FileDropzone.tsx | 2 +- .../components/InlineToast/InlineToast.tsx | 2 +- .../LoadingBar/LoadingBar.story.tsx | 2 +- .../src/components/ThemeDemos/ThemeDemo.tsx | 8 +-- 12 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 packages/grafana-eslint-rules/rules/no-border-radius-literal.cjs diff --git a/.eslintrc b/.eslintrc index 81112897a70..f0aae1abab2 100644 --- a/.eslintrc +++ b/.eslintrc @@ -7,6 +7,7 @@ "import/external-module-folders": ["node_modules", ".yarn"] }, "rules": { + "@grafana/no-border-radius-literal": "error", "react/prop-types": "off", // need to ignore emotion's `css` prop, see https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/no-unknown-property.md#rule-options "react/no-unknown-property": ["error", { "ignore": ["css"] }], diff --git a/packages/grafana-eslint-rules/README.md b/packages/grafana-eslint-rules/README.md index b497ed73814..de81df7801f 100644 --- a/packages/grafana-eslint-rules/README.md +++ b/packages/grafana-eslint-rules/README.md @@ -12,6 +12,12 @@ Previously we hijacked the aria-label property to use as E2E selectors as an att Now, we prefer using data-testid for E2E selectors. +### `@grafana/no-border-radius-literal` + +Check if border-radius theme tokens are used. + +To improve the consistency across Grafana we encourage devs to use tokens instead of custom values. In this case, we want the `borderRadius` to use the appropiate token such as `theme.shape.radius.default`, `theme.shape.radius.pill` or `theme.shape.radius.circle`. + ### `@grafana/theme-token-usage` Used to find all instances of `theme` tokens being used in the codebase and emit the counts as metrics. Should **not** be used as an actual lint rule! diff --git a/packages/grafana-eslint-rules/index.cjs b/packages/grafana-eslint-rules/index.cjs index 2ce0fe5ef27..581ca0b2e87 100644 --- a/packages/grafana-eslint-rules/index.cjs +++ b/packages/grafana-eslint-rules/index.cjs @@ -1,9 +1,11 @@ const noAriaLabelSelectors = require('./rules/no-aria-label-e2e-selectors.cjs'); +const noBorderRadiusLiteral = require('./rules/no-border-radius-literal.cjs'); const themeTokenUsage = require('./rules/theme-token-usage.cjs'); module.exports = { rules: { 'no-aria-label-selectors': noAriaLabelSelectors, + 'no-border-radius-literal': noBorderRadiusLiteral, 'theme-token-usage': themeTokenUsage, }, }; diff --git a/packages/grafana-eslint-rules/rules/no-border-radius-literal.cjs b/packages/grafana-eslint-rules/rules/no-border-radius-literal.cjs new file mode 100644 index 00000000000..35064ac3c14 --- /dev/null +++ b/packages/grafana-eslint-rules/rules/no-border-radius-literal.cjs @@ -0,0 +1,61 @@ +// @ts-check +const { ESLintUtils, AST_NODE_TYPES } = require('@typescript-eslint/utils'); + +const createRule = ESLintUtils.RuleCreator((name) => `https://github.com/grafana/grafana#${name}`); + +const borderRadiusRule = createRule({ + create(context) { + return { + CallExpression(node) { + if ( + node.callee.type === AST_NODE_TYPES.Identifier && + node.callee.name === 'css' + ) { + const cssObjects = node.arguments.flatMap((node) => { + switch (node.type) { + case AST_NODE_TYPES.ObjectExpression: + return [node]; + case AST_NODE_TYPES.ArrayExpression: + return node.elements.filter(v => v.type === AST_NODE_TYPES.ObjectExpression); + default: + return []; + } + }); + + for (const cssObject of cssObjects) { + if (cssObject.type === AST_NODE_TYPES.ObjectExpression) { + for (const property of cssObject.properties) { + if ( + property.type === AST_NODE_TYPES.Property && + property.key.type === AST_NODE_TYPES.Identifier && + property.key.name === 'borderRadius' && + property.value.type === AST_NODE_TYPES.Literal + ) { + context.report({ + node: property, + messageId: 'borderRadiusId', + }); + } + } + } + } + } + }, + }; + }, + name: 'no-border-radius-literal', + meta: { + type: 'problem', + docs: { + description: 'Check if border-radius theme tokens are used', + recommended: false, + }, + messages: { + borderRadiusId: 'Prefer using theme.shape.radius tokens instead of literal values.', + }, + schema: [], + }, + defaultOptions: [], +}); + +module.exports = borderRadiusRule; diff --git a/packages/grafana-ui/src/components/DateTimePickers/DatePicker/DatePicker.tsx b/packages/grafana-ui/src/components/DateTimePickers/DatePicker/DatePicker.tsx index 9f4098d000f..c2a7d8f6457 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/DatePicker/DatePicker.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/DatePicker/DatePicker.tsx @@ -71,7 +71,8 @@ export const getStyles = (theme: GrafanaTheme2) => { boxShadow: theme.shadows.z3, backgroundColor: theme.colors.background.primary, border: `1px solid ${theme.colors.border.weak}`, - borderRadius: '2px 0 0 2px', + borderTopLeftRadius: theme.shape.radius.default, + borderBottomLeftRadius: theme.shape.radius.default, 'button:disabled': { color: theme.colors.text.disabled, diff --git a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerCalendar.tsx b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerCalendar.tsx index 5e18b70bd8e..2e32795301e 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerCalendar.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerCalendar.tsx @@ -24,7 +24,8 @@ export const getStyles = (theme: GrafanaTheme2, isReversed = false) => { backgroundColor: theme.colors.background.primary, zIndex: -1, border: `1px solid ${theme.colors.border.weak}`, - borderRadius: '2px 0 0 2px', + borderTopLeftRadius: theme.shape.radius.default, + borderBottomLeftRadius: theme.shape.radius.default, '&:after': { display: 'block', diff --git a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerContent.tsx b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerContent.tsx index b21741b6d6a..7b68b579a7e 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerContent.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimePickerContent.tsx @@ -268,7 +268,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, isReversed, hideQuickRang background: theme.colors.background.primary, boxShadow: theme.shadows.z3, width: `${isFullscreen ? '546px' : '262px'}`, - borderRadius: '2px', + borderRadius: theme.shape.radius.default, border: `1px solid ${theme.colors.border.weak}`, [`${isReversed ? 'left' : 'right'}`]: 0, }), diff --git a/packages/grafana-ui/src/components/DateTimePickers/TimeZonePicker/TimeZoneOffset.tsx b/packages/grafana-ui/src/components/DateTimePickers/TimeZonePicker/TimeZoneOffset.tsx index 03e1f334298..0c1ca7fbb7d 100644 --- a/packages/grafana-ui/src/components/DateTimePickers/TimeZonePicker/TimeZoneOffset.tsx +++ b/packages/grafana-ui/src/components/DateTimePickers/TimeZonePicker/TimeZoneOffset.tsx @@ -52,7 +52,7 @@ const getStyles = (theme: GrafanaTheme2) => { color: theme.colors.text.primary, background: theme.colors.background.secondary, padding: '2px 5px', - borderRadius: '2px', + borderRadius: theme.shape.radius.default, marginLeft: '4px', }), }; diff --git a/packages/grafana-ui/src/components/FileDropzone/FileDropzone.tsx b/packages/grafana-ui/src/components/FileDropzone/FileDropzone.tsx index 4b540b76c1c..ce4746c24ba 100644 --- a/packages/grafana-ui/src/components/FileDropzone/FileDropzone.tsx +++ b/packages/grafana-ui/src/components/FileDropzone/FileDropzone.tsx @@ -303,7 +303,7 @@ function getStyles(theme: GrafanaTheme2, isDragActive?: boolean) { flexDirection: 'column', width: '100%', padding: theme.spacing(2), - borderRadius: '2px', + borderRadius: theme.shape.radius.default, border: `1px dashed ${theme.colors.border.strong}`, backgroundColor: isDragActive ? theme.colors.background.secondary : theme.colors.background.primary, cursor: 'pointer', diff --git a/packages/grafana-ui/src/components/InlineToast/InlineToast.tsx b/packages/grafana-ui/src/components/InlineToast/InlineToast.tsx index 8488682a3d2..6003f2f2093 100644 --- a/packages/grafana-ui/src/components/InlineToast/InlineToast.tsx +++ b/packages/grafana-ui/src/components/InlineToast/InlineToast.tsx @@ -63,7 +63,7 @@ const getStyles = (theme: GrafanaTheme2) => { background: theme.components.tooltip.background, color: theme.components.tooltip.text, padding: theme.spacing(0.5, 1.5), // get's an extra .5 of vertical padding to account for the rounded corners - borderRadius: 100, // just a sufficiently large value to ensure ends are completely rounded + borderRadius: theme.shape.radius.circle, display: 'inline-flex', gap: theme.spacing(0.5), alignItems: 'center', diff --git a/packages/grafana-ui/src/components/LoadingBar/LoadingBar.story.tsx b/packages/grafana-ui/src/components/LoadingBar/LoadingBar.story.tsx index c5065adcc9d..876aa57f7e0 100644 --- a/packages/grafana-ui/src/components/LoadingBar/LoadingBar.story.tsx +++ b/packages/grafana-ui/src/components/LoadingBar/LoadingBar.story.tsx @@ -31,7 +31,7 @@ const getStyles = (theme: GrafanaTheme2) => { width: '400px', height: '200px', border: `1px solid ${borderColor}`, - borderRadius: '3px', + borderRadius: theme.shape.radius.default, }), }; }; diff --git a/packages/grafana-ui/src/components/ThemeDemos/ThemeDemo.tsx b/packages/grafana-ui/src/components/ThemeDemos/ThemeDemo.tsx index c899a907bff..377f14beba3 100644 --- a/packages/grafana-ui/src/components/ThemeDemos/ThemeDemo.tsx +++ b/packages/grafana-ui/src/components/ThemeDemos/ThemeDemo.tsx @@ -230,7 +230,7 @@ export function RichColorDemo({ theme, color }: RichColorDemoProps) {
@@ -255,7 +255,7 @@ export function RichColorDemo({ theme, color }: RichColorDemoProps) {
@@ -267,7 +267,7 @@ export function RichColorDemo({ theme, color }: RichColorDemoProps) { className={css({ border: `1px solid ${color.border}`, color: color.text, - borderRadius: '4px', + borderRadius: theme.shape.radius.default, padding: '8px', })} >