Border radius: Improve rule and fix remaining violations (#104569)
* improve rule and fix remaining borderRadius violations * prettier * Add test case for nested classes * Fix unnecessary string wrapping --------- Co-authored-by: Tom Ratcliffe <tom.ratcliffe@grafana.com>
This commit is contained in:
co-authored by
Tom Ratcliffe
parent
77a2b34b02
commit
b56a4a5295
@@ -48,6 +48,7 @@ module.exports = [
|
||||
'scripts/grafana-server/tmp',
|
||||
'!.betterer.eslint.config.js',
|
||||
'packages/grafana-ui/src/graveyard', // deprecated UI components slated for removal
|
||||
'public/build-swagger', // swagger build output
|
||||
],
|
||||
},
|
||||
// Conditionally run the betterer rules if enabled in dev's config
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
// @ts-check
|
||||
const { ESLintUtils, AST_NODE_TYPES } = require('@typescript-eslint/utils');
|
||||
|
||||
const createRule = ESLintUtils.RuleCreator(
|
||||
@@ -8,36 +7,17 @@ const createRule = ESLintUtils.RuleCreator(
|
||||
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 [];
|
||||
}
|
||||
[`${AST_NODE_TYPES.CallExpression}[callee.name="css"] ${AST_NODE_TYPES.Property}`]: function (node) {
|
||||
if (
|
||||
node.type === AST_NODE_TYPES.Property &&
|
||||
node.key.type === AST_NODE_TYPES.Identifier &&
|
||||
node.key.name === 'borderRadius' &&
|
||||
node.value.type === AST_NODE_TYPES.Literal
|
||||
) {
|
||||
context.report({
|
||||
node,
|
||||
messageId: 'borderRadiusId',
|
||||
});
|
||||
|
||||
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',
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
@@ -14,6 +14,10 @@ RuleTester.setDefaultConfig({
|
||||
},
|
||||
});
|
||||
|
||||
const expectedError = {
|
||||
messageId: 'borderRadiusId',
|
||||
};
|
||||
|
||||
const ruleTester = new RuleTester();
|
||||
|
||||
ruleTester.run('eslint no-border-radius-literal', noBorderRadiusLiteral, {
|
||||
@@ -32,27 +36,27 @@ ruleTester.run('eslint no-border-radius-literal', noBorderRadiusLiteral, {
|
||||
invalid: [
|
||||
{
|
||||
code: `css({ borderRadius: '2px' })`,
|
||||
errors: [
|
||||
{
|
||||
message: 'Prefer using theme.shape.radius tokens instead of literal values.',
|
||||
},
|
||||
],
|
||||
errors: [expectedError],
|
||||
},
|
||||
{
|
||||
code: `css({ lineHeight: 1 }, { borderRadius: '2px' })`,
|
||||
errors: [
|
||||
{
|
||||
message: 'Prefer using theme.shape.radius tokens instead of literal values.',
|
||||
},
|
||||
],
|
||||
errors: [expectedError],
|
||||
},
|
||||
{
|
||||
code: `css([{ lineHeight: 1 }, { borderRadius: '2px' }])`,
|
||||
errors: [
|
||||
{
|
||||
message: 'Prefer using theme.shape.radius tokens instead of literal values.',
|
||||
},
|
||||
],
|
||||
errors: [expectedError],
|
||||
},
|
||||
{
|
||||
name: 'nested classes',
|
||||
code: `
|
||||
css({
|
||||
foo: {
|
||||
nested: {
|
||||
borderRadius: '100px',
|
||||
},
|
||||
},
|
||||
})`,
|
||||
errors: [expectedError],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
@@ -302,8 +302,7 @@ const getStyles = (theme: GrafanaTheme2) => ({
|
||||
display: 'inline-block',
|
||||
width: '10px',
|
||||
height: '10px',
|
||||
// eslint-disable-next-line @grafana/no-border-radius-literal
|
||||
borderRadius: '50%',
|
||||
borderRadius: theme.shape.radius.circle,
|
||||
}),
|
||||
colorDotDiff: css({
|
||||
label: 'colorDotDiff',
|
||||
|
||||
@@ -169,7 +169,7 @@ export const getBodyStyles = (theme: GrafanaTheme2) => {
|
||||
|
||||
abbr: {
|
||||
backgroundColor: theme.colors.primary.main,
|
||||
borderRadius: '100px',
|
||||
borderRadius: theme.shape.radius.pill,
|
||||
display: 'block',
|
||||
paddingTop: '2px',
|
||||
height: '26px',
|
||||
|
||||
@@ -1002,6 +1002,7 @@ const getStyles = (theme: GrafanaTheme2) => ({
|
||||
},
|
||||
'::-webkit-scrollbar-thumb': {
|
||||
backgroundColor: 'rgba(204, 204, 220, 0.16)',
|
||||
// eslint-disable-next-line @grafana/no-border-radius-literal
|
||||
borderRadius: '4px',
|
||||
},
|
||||
'::-webkit-scrollbar-track': {
|
||||
|
||||
@@ -21,7 +21,7 @@ export function getCardStyles(theme: GrafanaTheme2) {
|
||||
background: theme.colors.background.secondary,
|
||||
boxShadow: 'none',
|
||||
padding: theme.spacing(2),
|
||||
borderRadius: '4px',
|
||||
borderRadius: theme.shape.radius.default,
|
||||
|
||||
'&:hover': {
|
||||
background: theme.colors.emphasize(theme.colors.background.secondary, 0.03),
|
||||
@@ -158,7 +158,7 @@ export function getCardStyles(theme: GrafanaTheme2) {
|
||||
},
|
||||
|
||||
'.card-item': {
|
||||
borderRadius: '2px',
|
||||
borderRadius: theme.shape.radius.default,
|
||||
},
|
||||
|
||||
'.card-item-header': {
|
||||
|
||||
@@ -10,7 +10,7 @@ export function getCodeStyles(theme: GrafanaTheme2) {
|
||||
backgroundColor: theme.colors.background.primary,
|
||||
color: theme.colors.text.primary,
|
||||
border: `1px solid ${theme.colors.border.medium}`,
|
||||
borderRadius: '4px',
|
||||
borderRadius: theme.shape.radius.default,
|
||||
},
|
||||
|
||||
code: {
|
||||
|
||||
@@ -113,7 +113,7 @@ export function getDashboardGridStyles(theme: GrafanaTheme2) {
|
||||
'&:is(:hover),&:not(:hover)': {
|
||||
outline: `2px solid ${theme.colors.primary.border}`,
|
||||
outlineOffset: '0px',
|
||||
borderRadius: '2px',
|
||||
borderRadius: theme.shape.radius.default,
|
||||
},
|
||||
},
|
||||
|
||||
|
||||
@@ -142,7 +142,7 @@ export function getDashDiffStyles(theme: GrafanaTheme2) {
|
||||
|
||||
'.diff-label': {
|
||||
backgroundColor: theme.colors.action.hover,
|
||||
borderRadius: '3px',
|
||||
borderRadius: theme.shape.radius.default,
|
||||
color: theme.colors.text.primary,
|
||||
display: 'inline',
|
||||
fontSize: `${theme.typography.fontSize}px`,
|
||||
|
||||
@@ -276,6 +276,7 @@ export function getElementStyles(theme: GrafanaTheme2, isExtensionSidebarOpen?:
|
||||
// 2. Correct font properties not being inherited.
|
||||
// 3. Address margins set differently in Firefox 4+, Safari, and Chrome.
|
||||
'button, input, optgroup, select, textarea': {
|
||||
// eslint-disable-next-line @grafana/no-border-radius-literal
|
||||
borderRadius: 0,
|
||||
color: 'inherit',
|
||||
font: 'inherit',
|
||||
|
||||
@@ -66,7 +66,7 @@ export function getFilterTableStyles(theme: GrafanaTheme2) {
|
||||
'.filter-table__avatar': {
|
||||
width: '25px',
|
||||
height: '25px',
|
||||
borderRadius: '50%',
|
||||
borderRadius: theme.shape.radius.circle,
|
||||
},
|
||||
|
||||
'&--hover': {
|
||||
|
||||
@@ -32,8 +32,7 @@ const getDotStyles = (theme: GrafanaTheme2, props: DotStylesProps) => {
|
||||
width: size,
|
||||
height: size,
|
||||
|
||||
// eslint-disable-next-line @grafana/no-border-radius-literal
|
||||
borderRadius: '100%',
|
||||
borderRadius: theme.shape.radius.circle,
|
||||
|
||||
backgroundColor: theme.colors.secondary.main,
|
||||
outline: `solid ${outlineSize} ${theme.colors.secondary.transparent}`,
|
||||
|
||||
@@ -273,7 +273,7 @@ const ContactPointReceiverMetadataRow = ({ diagnostics, sendingResolved }: Conta
|
||||
|
||||
const getStyles = (theme: GrafanaTheme2) => ({
|
||||
contactPointWrapper: css({
|
||||
borderRadius: `${theme.shape.radius.default}`,
|
||||
borderRadius: theme.shape.radius.default,
|
||||
border: `solid 1px ${theme.colors.border.weak}`,
|
||||
borderBottom: 'none',
|
||||
}),
|
||||
|
||||
-29
@@ -86,35 +86,6 @@ export const TransformationEditor = ({
|
||||
|
||||
const getStyles = (theme: GrafanaTheme2) => {
|
||||
return {
|
||||
title: css({
|
||||
display: 'flex',
|
||||
padding: '4px 8px 4px 8px',
|
||||
position: 'relative',
|
||||
height: '35px',
|
||||
// eslint-disable-next-line @grafana/no-border-radius-literal
|
||||
borderRadius: '4px 4px 0 0',
|
||||
flexWrap: 'nowrap',
|
||||
justifyContent: 'space-between',
|
||||
alignItems: 'center',
|
||||
}),
|
||||
name: css({
|
||||
fontWeight: theme.typography.fontWeightMedium,
|
||||
color: theme.colors.primary.text,
|
||||
}),
|
||||
iconRow: css({
|
||||
display: 'flex',
|
||||
}),
|
||||
icon: css({
|
||||
background: 'transparent',
|
||||
border: 'none',
|
||||
boxShadow: 'none',
|
||||
cursor: 'pointer',
|
||||
color: theme.colors.text.secondary,
|
||||
marginLeft: theme.spacing(1),
|
||||
'&:hover': {
|
||||
color: theme.colors.text.primary,
|
||||
},
|
||||
}),
|
||||
debugWrapper: css({
|
||||
display: 'flex',
|
||||
flexDirection: 'row',
|
||||
|
||||
@@ -157,7 +157,7 @@ export function getGlobalStyles(theme: GrafanaTheme2) {
|
||||
'&.rc-tree-checkbox-indeterminate.rc-tree-checkbox-disabled': {
|
||||
position: 'relative',
|
||||
background: '#ccc',
|
||||
borderRadius: '3px',
|
||||
borderRadius: theme.shape.radius.default,
|
||||
'&::after': {
|
||||
position: 'absolute',
|
||||
top: '5px',
|
||||
|
||||
@@ -263,6 +263,7 @@ export const getStyles = (theme: GrafanaTheme2, isResizeInProgress: boolean) =>
|
||||
background: theme.colors.background.primary,
|
||||
},
|
||||
'::-webkit-scrollbar-thumb': {
|
||||
// eslint-disable-next-line @grafana/no-border-radius-literal
|
||||
borderRadius: '10px',
|
||||
},
|
||||
'::-webkit-scrollbar-corner': {
|
||||
|
||||
Reference in New Issue
Block a user