Skip to content

Commit df74628

Browse files
coryrylancoryrylan
committed
fix(lint): update no-missing-gap-space rule to allow gap-optional values
- Added support for layouts with gap-optional values in the no-missing-gap-space rule. - Updated ESLint configurations to disable the rule for specific test files. - Adjusted various components to comply with the new gap requirements. Signed-off-by: Cory Rylan <crylan@nvidia.com>
1 parent a704fbe commit df74628

4 files changed

Lines changed: 95 additions & 64 deletions

File tree

‎projects/lint/src/eslint/configs/html.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export const elementsHtmlConfig: Linter.Config = {
126126
'@nvidia-elements/lint/no-unstyled-typography': ['error'],
127127
'@nvidia-elements/lint/no-tailwind-classes': ['error'],
128128
'@nvidia-elements/lint/prefer-aria-label-in-compact-containers': ['error'],
129-
'@nvidia-elements/lint/no-unexpected-style-customization': ['off'],
130-
'@nvidia-elements/lint/no-missing-gap-space': ['off']
129+
'@nvidia-elements/lint/no-missing-gap-space': ['error'],
130+
'@nvidia-elements/lint/no-unexpected-style-customization': ['off']
131131
}
132132
};

‎projects/lint/src/eslint/internals/index.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,4 +135,14 @@ describe('lintPlaygroundTemplate', () => {
135135

136136
expect(tailwindMessage?.severity).toBe('error');
137137
});
138+
139+
it('should apply missing gap checks to strict templates', async () => {
140+
const missingGap = '<div nve-layout="row"><span>one</span><span>two</span></div>';
141+
const explicitNoGap = '<div nve-layout="row gap:none"><span>one</span><span>two</span></div>';
142+
const strictResult = await lintTemplate(missingGap, { strict: true });
143+
const explicitNoGapResult = await lintTemplate(explicitNoGap, { strict: true });
144+
145+
expect(strictResult.find(message => message.id === 'missing-gap-space')?.severity).toBe('error');
146+
expect(explicitNoGapResult.some(message => message.id === 'missing-gap-space')).toBe(false);
147+
});
138148
});

‎projects/lint/src/eslint/rules/no-missing-gap-space.test.ts‎

Lines changed: 29 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,21 @@ import noMissingGapSpace from './no-missing-gap-space.js';
1010
const rule = noMissingGapSpace as unknown as JSRuleDefinition;
1111

1212
const SUGGESTED_GAP_SIZES = ['xs', 'sm', 'md', 'lg', 'xl'];
13+
const GAP_OPTIONAL_VALUES = [
14+
'full',
15+
'gap:none',
16+
'align:center',
17+
'align:horizontal-center',
18+
'align:vertical-center',
19+
'align:stretch',
20+
'align:horizontal-stretch',
21+
'align:vertical-stretch',
22+
'align:left',
23+
'align:right',
24+
'align:space-around',
25+
'align:space-between',
26+
'align:space-evenly'
27+
];
1328

1429
function gapSuggestions(layout: string) {
1530
return SUGGESTED_GAP_SIZES.map(size => ({
@@ -43,7 +58,7 @@ describe('noMissingGapSpace', () => {
4358
expect(noMissingGapSpace.meta.docs.category).toBe('Best Practice');
4459
expect(noMissingGapSpace.meta.docs.recommended).toBe(true);
4560
expect(noMissingGapSpace.meta.docs.url).toContain('/docs/lint/');
46-
expect(noMissingGapSpace.meta.schema).toBeDefined();
61+
expect(noMissingGapSpace.meta.schema).toEqual([]);
4762
expect(noMissingGapSpace.meta.messages).toBeDefined();
4863
expect(noMissingGapSpace.meta.messages['missing-gap-space']).toBe(
4964
`Layout "{{layout}}" is missing gap spacing. Add a gap value such as "${SUGGESTED_GAP_SIZES.join('", "')}"`
@@ -73,28 +88,11 @@ describe('noMissingGapSpace', () => {
7388
});
7489
});
7590

76-
it('should allow layouts with spacing alignment values', () => {
77-
tester.run('should allow layouts with spacing alignment values', rule, {
78-
valid: [
79-
'<div nve-layout="row align:space-between"></div>',
80-
'<div nve-layout="row align:space-around"></div>',
81-
'<div nve-layout="row align:space-evenly"></div>',
82-
'<div nve-layout="column align:space-between"></div>',
83-
'<div nve-layout="column align:space-around"></div>',
84-
'<div nve-layout="column align:space-evenly"></div>'
85-
],
86-
invalid: []
87-
});
88-
});
89-
90-
it('should allow non-row/column layouts without gap', () => {
91-
tester.run('should allow non-row/column layouts without gap', rule, {
92-
valid: [
93-
'<div nve-layout="grid"></div>',
94-
'<div nve-layout="grid gap:md"></div>',
95-
'<div nve-layout="full"></div>',
96-
'<div nve-layout="grid span-items:4"></div>'
97-
],
91+
it('should allow layouts with gap-optional values', () => {
92+
tester.run('should allow layouts with gap-optional values', rule, {
93+
valid: ['row', 'column', 'grid'].flatMap(layout =>
94+
GAP_OPTIONAL_VALUES.map(value => `<div nve-layout="${layout} ${value}"></div>`)
95+
),
9896
invalid: []
9997
});
10098
});
@@ -154,17 +152,17 @@ describe('noMissingGapSpace', () => {
154152
});
155153
});
156154

157-
it('should report missing gap on row layout with other modifiers', () => {
158-
tester.run('should report missing gap on row layout with modifiers', rule, {
155+
it('should report missing gap on row layout with non-exempt alignment', () => {
156+
tester.run('should report missing gap on row layout with non-exempt alignment', rule, {
159157
valid: [],
160158
invalid: [
161159
{
162-
code: '<div nve-layout="row align:center"></div>',
160+
code: '<div nve-layout="row align:top"></div>',
163161
errors: [
164162
{
165163
messageId: 'missing-gap-space',
166-
data: { layout: 'row align:center' },
167-
suggestions: gapSuggestions('row align:center')
164+
data: { layout: 'row align:top' },
165+
suggestions: gapSuggestions('row align:top')
168166
}
169167
]
170168
}
@@ -177,12 +175,12 @@ describe('noMissingGapSpace', () => {
177175
valid: [],
178176
invalid: [
179177
{
180-
code: '<div nve-layout="column full pad:md"></div>',
178+
code: '<div nve-layout="column pad:md"></div>',
181179
errors: [
182180
{
183181
messageId: 'missing-gap-space',
184-
data: { layout: 'column full pad:md' },
185-
suggestions: gapSuggestions('column full pad:md')
182+
data: { layout: 'column pad:md' },
183+
suggestions: gapSuggestions('column pad:md')
186184
}
187185
]
188186
}

‎projects/lint/src/eslint/rules/no-missing-gap-space.ts‎

Lines changed: 54 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,60 @@ import type { Rule } from 'eslint';
55
import { createVisitors } from '@html-eslint/eslint-plugin/lib/rules/utils/visitors.js';
66
import { findAttr } from '@html-eslint/eslint-plugin/lib/rules/utils/node.js';
77
import { VALUE_BINDINGS } from '../internals/attributes.js';
8-
import type { HtmlTagNode } from '../rule-types.js';
8+
import type { HtmlAttribute, HtmlTagNode } from '../rule-types.js';
99

1010
declare const __ELEMENTS_PAGES_BASE_URL__: string;
11-
/** Spacing alignment values that manage their own distribution and override gap */
12-
const SPACING_ALIGNMENTS = ['align:space-around', 'align:space-between', 'align:space-evenly'];
11+
const GAP_OPTIONAL_VALUES = new Set([
12+
'full',
13+
'align:center',
14+
'align:horizontal-center',
15+
'align:vertical-center',
16+
'align:stretch',
17+
'align:horizontal-stretch',
18+
'align:vertical-stretch',
19+
'align:left',
20+
'align:right',
21+
'align:space-around',
22+
'align:space-between',
23+
'align:space-evenly'
24+
]);
1325

1426
/** Context limited gap sizes to suggest as fixes */
1527
const SUGGESTED_GAP_SIZES = ['xs', 'sm', 'md', 'lg', 'xl'];
1628

29+
function hasGapOptionalValue(values: string[]): boolean {
30+
return values.some(value => GAP_OPTIONAL_VALUES.has(value));
31+
}
32+
33+
function suggestedLayout(value: string, size: string): string {
34+
return `${value} gap:${size}`;
35+
}
36+
37+
function reportGapViolation({
38+
context,
39+
layoutAttr,
40+
value
41+
}: {
42+
context: Rule.RuleContext;
43+
layoutAttr: HtmlAttribute;
44+
value: string;
45+
}) {
46+
context.report({
47+
node: layoutAttr,
48+
messageId: 'missing-gap-space',
49+
data: { layout: value },
50+
suggest: SUGGESTED_GAP_SIZES.map(size => ({
51+
messageId: 'suggest-add-gap',
52+
data: { size },
53+
fix: (fixer: Rule.RuleFixer) =>
54+
fixer.replaceText(
55+
layoutAttr as unknown as Rule.Node,
56+
`nve-layout=${layoutAttr.startWrapper?.value}${suggestedLayout(value, size)}${layoutAttr.endWrapper?.value}`
57+
)
58+
}))
59+
});
60+
}
61+
1762
const rule = {
1863
meta: {
1964
type: 'problem' as const,
@@ -36,36 +81,14 @@ const rule = {
3681
const layoutAttr = findAttr(node, 'nve-layout');
3782
if (!layoutAttr) return;
3883

39-
const value = layoutAttr.value?.value ?? '';
40-
84+
const value: string = layoutAttr.value?.value ?? '';
4185
if (VALUE_BINDINGS.some(binding => value.includes(binding))) return;
4286

43-
const segments = value.split(' ').filter((s: string) => s !== '');
44-
45-
const isRowOrColumn = segments.includes('row') || segments.includes('column');
46-
if (!isRowOrColumn) return;
47-
48-
const hasGap = segments.some((s: string) => s.startsWith('gap:'));
49-
if (hasGap) return;
50-
51-
const hasSpacingAlignment = segments.some((s: string) => SPACING_ALIGNMENTS.includes(s));
52-
if (hasSpacingAlignment) return;
53-
54-
context.report({
55-
node: layoutAttr,
56-
messageId: 'missing-gap-space',
57-
data: { layout: value },
58-
suggest: SUGGESTED_GAP_SIZES.map(size => ({
59-
messageId: 'suggest-add-gap',
60-
data: { size },
61-
fix: fixer => {
62-
return fixer.replaceText(
63-
layoutAttr,
64-
`nve-layout=${layoutAttr.startWrapper.value}${value} gap:${size}${layoutAttr.endWrapper.value}`
65-
);
66-
}
67-
}))
68-
});
87+
const values = value.split(/\s+/).filter(Boolean);
88+
const isLayout = values.includes('row') || values.includes('column') || values.includes('grid');
89+
const hasGap = values.some(segment => segment.startsWith('gap:'));
90+
if (!isLayout || hasGap || hasGapOptionalValue(values)) return;
91+
reportGapViolation({ context, layoutAttr, value });
6992
}
7093
});
7194
}

0 commit comments

Comments
 (0)