Skip to content

Commit 56e4701

Browse files
committed
fix(lint): enhance data binding checks and update test cases for popover triggers
- Improved the `hasDataBinding` function for better readability and efficiency. - Refactored the way attributes are checked for Angular and Lit property bindings. - Updated test cases to reflect changes in property names for `popoverTargetElement`, `commandForElement`, and `interestForElement`. Signed-off-by: Cory Rylan <crylan@nvidia.com>
1 parent b7be2d9 commit 56e4701

2 files changed

Lines changed: 34 additions & 30 deletions

File tree

‎projects/lint/src/eslint/rules/no-missing-popover-trigger.test.ts‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ describe('noMissingPopoverTrigger', () => {
123123
});
124124

125125
it('should report popover elements with template syntax content but no trigger',()=>{
126-
// Template syntax in content does not affect trigger requirement
126+
// Content template syntax still requires a trigger
127127
tester.run('template syntax content requires trigger',rule,{
128128
valid: [],
129129
invalid: [
@@ -171,24 +171,30 @@ describe('noMissingPopoverTrigger', () => {
171171
// Template literal in popovertarget - popover should pass because trigger could match
172172
`<nve-button popovertarget="\${targetId}">Open</nve-button>
173173
<nve-dropdown id="dropdown">Content</nve-dropdown>`,
174-
// Angular binding in popovertarget
175-
`<nve-button [popovertarget]="targetId">Open</nve-button>
174+
// Angular property binding for popovertarget
175+
`<nve-button [popoverTargetElement]="target">Open</nve-button>
176176
<nve-dropdown id="dropdown">Content</nve-dropdown>`,
177-
// Lit binding in popovertarget
178-
`<nve-button .popovertarget="\${targetId}">Open</nve-button>
177+
// Lit property binding for popovertarget
178+
`<nve-button .popoverTargetElement="\${target}">Open</nve-button>
179179
<nve-dropdown id="dropdown">Content</nve-dropdown>`,
180180
// Template literal in commandfor
181181
`<nve-button commandfor="\${targetId}">Open</nve-button>
182182
<nve-dropdown id="dropdown">Content</nve-dropdown>`,
183-
// Angular binding in commandfor
184-
`<nve-button [commandfor]="targetId">Open</nve-button>
183+
// Angular property binding for commandfor
184+
`<nve-button [commandForElement]="target">Open</nve-button>
185185
<nve-dropdown id="dropdown">Content</nve-dropdown>`,
186-
// Lit binding in commandfor
187-
`<nve-button .commandfor="\${targetId}">Open</nve-button>
186+
// Lit property binding for commandfor
187+
`<nve-button .commandForElement="\${target}">Open</nve-button>
188+
<nve-dropdown id="dropdown">Content</nve-dropdown>`,
189+
// Lit property binding for interestfor
190+
`<nve-button .interestForElement="\${target}">Open</nve-button>
188191
<nve-dropdown id="dropdown">Content</nve-dropdown>`,
189192
// JSX expression in popovertarget
190-
`<nve-button popovertarget="{targetId}">Open</nve-button>
193+
`<nve-button popovertarget={targetId}>Open</nve-button>
191194
<nve-dropdown id="dropdown">Content</nve-dropdown>`,
195+
// JSX property binding for an HTMLElement ref
196+
`<nve-button popoverTargetElement={popoverRef.current}>Open</nve-button>
197+
<nve-dropdown ref={popoverRef} id="dropdown">Content</nve-dropdown>`,
192198
// Vue/Handlebars in popovertarget
193199
`<nve-button popovertarget="{{targetId}}">Open</nve-button>
194200
<nve-dropdown id="dropdown">Content</nve-dropdown>`

‎projects/lint/src/eslint/rules/no-missing-popover-trigger.ts‎

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ const DATA_BINDING_PATTERNS = [
2121
* Check if an attribute value contains data binding syntax.
2222
*/
2323
functionhasDataBinding(value: string|undefined): boolean{
24-
if(!value)returnfalse;
25-
returnDATA_BINDING_PATTERNS.some(pattern=>pattern.test(value));
24+
return!!value&&DATA_BINDING_PATTERNS.some(pattern=>pattern.test(value));
2625
}
2726

2827
/**
@@ -31,30 +30,25 @@ function hasDataBinding(value: string | undefined): boolean {
3130
*/
3231
functionfindAttrWithBinding(
3332
node: HtmlNode,
34-
attrName: string
33+
attrName: string,
34+
propertyName=attrName
3535
): {attr: ReturnType<typeoffindAttr>;hasBinding: boolean}{
3636
// Check for standard attribute
3737
conststandard=findAttr(node,attrName);
3838
if(standard){
3939
return{attr: standard,hasBinding: hasDataBinding(standard.value?.value)};
4040
}
4141

42-
// Check for Angular property binding: [attrName]
43-
constangularAttr=findAttr(node,`[${attrName}]`);
44-
if(angularAttr){
45-
return{attr: angularAttr,hasBinding: true};
42+
constjsxProperty=findAttr(node,propertyName);
43+
if(jsxProperty&&hasDataBinding(jsxProperty.value?.value)){
44+
return{attr: jsxProperty,hasBinding: true};
4645
}
4746

48-
// Check for Lit property binding: .attrName
49-
constlitAttr=findAttr(node,`.${attrName}`);
50-
if(litAttr){
51-
return{attr: litAttr,hasBinding: true};
52-
}
53-
54-
// Check for Lit boolean-attribute binding: ?attrName
55-
constlitBoolAttr=findAttr(node,`?${attrName}`);
56-
if(litBoolAttr){
57-
return{attr: litBoolAttr,hasBinding: true};
47+
for(constnameof[`[${propertyName}]`,`.${propertyName}`,`?${attrName}`]){
48+
constattr=findAttr(node,name);
49+
if(attr){
50+
return{ attr,hasBinding: true};
51+
}
5852
}
5953

6054
return{attr: undefined,hasBinding: false};
@@ -77,7 +71,11 @@ const POPOVER_ELEMENTS = [
7771
/**
7872
* Attributes that reference a popover target.
7973
*/
80-
constTRIGGER_ATTRIBUTES=['popovertarget','commandfor','interestfor']asconst;
74+
constTRIGGER_APIS=[
75+
['popovertarget','popoverTargetElement'],
76+
['commandfor','commandForElement'],
77+
['interestfor','interestForElement']
78+
]asconst;
8179

8280
interfacePopoverNode{
8381
node: HtmlNode;
@@ -114,8 +112,8 @@ const rule = {
114112
lethasDynamicTrigger=false;
115113

116114
functioncollectTriggerInfo(node: HtmlNode){
117-
for(constattrofTRIGGER_ATTRIBUTES){
118-
const{attr: found, hasBinding }=findAttrWithBinding(node,attr);
115+
for(const[attribute,property]ofTRIGGER_APIS){
116+
const{attr: found, hasBinding }=findAttrWithBinding(node,attribute,property);
119117
if(!found)continue;
120118
if(hasBinding){
121119
hasDynamicTrigger=true;

0 commit comments

Comments
 (0)