Skip to content

Commit 378e940

Browse files
daKmoRdaKmoR
committed
fix(fieldset): outside click only effects focused groups
1 parent c984a66 commit 378e940

File tree

3 files changed

+121
-50
lines changed

3 files changed

+121
-50
lines changed

‎packages/fieldset/src/LionFieldset.js‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -101,38 +101,25 @@ export class LionFieldset extends FormRegistrarMixin(
101101
this.__createTypeAbsenceValidators();
102102

103103
this._checkForOutsideClick = this._checkForOutsideClick.bind(this);
104-
}
105-
106-
connectedCallback() {
107-
// eslint-disable-next-line wc/guard-super-call
108-
super.connectedCallback();
109104

110-
this.addEventListener('focusin', this._updateTouchedClass);
111-
this.addEventListener('focusout', this._onFocusOut);
112105
this.addEventListener('focusin', this._syncFocused);
113-
106+
this.addEventListener('focusout', this._onFocusOut);
114107
this.addEventListener('validation-done', this.__validate);
115108
this.addEventListener('dirty-changed', this._syncDirty);
116-
117-
this._setRole();
118-
document.addEventListener('click', this._checkForOutsideClick);
119109
}
120110

121-
_checkForOutsideClick(event) {
122-
const outsideGroupClicked = !this.contains(event.target);
123-
if (outsideGroupClicked) {
124-
this.touched = true;
125-
}
111+
connectedCallback() {
112+
super.connectedCallback(); // eslint-disable-line wc/guard-super-call
113+
this._setRole();
126114
}
127115

128116
disconnectedCallback() {
129-
// eslint-disable-next-line wc/guard-super-call
130-
super.disconnectedCallback();
131-
this.removeEventListener('validation-done', this.__validate);
132-
this.removeEventListener('touched-changed', this._updateTouched);
133-
this.removeEventListener('dirty-changed', this._syncDirty);
117+
super.disconnectedCallback(); // eslint-disable-line wc/guard-super-call
134118

135-
document.removeEventListener('click', this._checkForOutsideClick);
119+
if (this.__hasActiveOutsideClickHandling) {
120+
document.removeEventListener('click', this._checkForOutsideClick);
121+
this.__hasActiveOutsideClickHandling = false;
122+
}
136123
}
137124

138125
updated(changedProps) {
@@ -162,6 +149,23 @@ export class LionFieldset extends FormRegistrarMixin(
162149
if (changedProps.has('focused')) {
163150
/** @deprecated use touched attribute instead */
164151
this.classList[this.focused ? 'add' : 'remove']('state-focused');
152+
if (this.focused === true) {
153+
this.__setupOutsideClickHandling();
154+
}
155+
}
156+
}
157+
158+
__setupOutsideClickHandling() {
159+
if (!this.__hasActiveOutsideClickHandling) {
160+
document.addEventListener('click', this._checkForOutsideClick);
161+
this.__hasActiveOutsideClickHandling = true;
162+
}
163+
}
164+
165+
_checkForOutsideClick(event) {
166+
const outsideGroupClicked = !this.contains(event.target);
167+
if (outsideGroupClicked) {
168+
this.touched = true;
165169
}
166170
}
167171

‎packages/fieldset/stories/index.stories.js‎

Lines changed: 82 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -84,23 +84,63 @@ storiesOf('Forms|Fieldset', module)
8484
</lion-fieldset>
8585
`,
8686
)
87+
.add('Validation', () => {
88+
function isDemoValidator() {
89+
return false;
90+
}
91+
92+
const demoValidator = (...factoryParams) => [
93+
(...params) => ({ validator: isDemoValidator(...params) }),
94+
...factoryParams,
95+
];
96+
97+
try {
98+
localize.addData('en-GB', 'lion-validate+validator', {
99+
error: {
100+
validator: 'Demo error message',
101+
},
102+
});
103+
} catch (error) {
104+
// expected as it's a demo
105+
}
106+
107+
return html`
108+
<lion-fieldset id="someId" .errorValidators=${[demoValidator()]}>
109+
<lion-input name="input1" label="Label"></lion-input>
110+
<button
111+
@click=${() => {
112+
document.getElementById('someId').serializeGroup();
113+
}}
114+
>
115+
Submit
116+
</button>
117+
</lion-fieldset>
118+
119+
<br />
120+
<br />
121+
<button>
122+
Tab-able
123+
</button>
124+
`;
125+
})
87126
.add('Validation 2 inputs', () => {
88-
const input1IsTen = value => ({
89-
input1IsTen: value.input1 === 'cats' && value.input2 === 'dogs',
127+
const isCatsAndDogs = value => ({
128+
isCatsAndDogs: value.input1 === 'cats' && value.input2 === 'dogs',
90129
});
91130
localize.locale = 'en-GB';
92131
try {
93-
localize.addData('en-GB', 'lion-validate+input1IsTen', {
132+
localize.addData('en-GB', 'lion-validate+isCatsAndDogs', {
94133
error: {
95-
input1IsTen: 'Input 1 needs to be "cats" and Input 2 needs to be "dogs"',
134+
isCatsAndDogs:
135+
'[Fieldset Error] Input 1 needs to be "cats" and Input 2 needs to be "dogs"',
96136
},
97137
});
98138
} catch (error) {
99139
// expected as it's a demo
100140
}
101141

102142
return html`
103-
<lion-fieldset .errorValidators=${[[input1IsTen]]}>
143+
<lion-fieldset .errorValidators=${[[isCatsAndDogs]]}>
104144
<lion-input
105145
label="An all time YouTube favorite"
106146
name="input1"
@@ -116,42 +156,56 @@ storiesOf('Forms|Fieldset', module)
116156
</lion-fieldset>
117157
`;
118158
})
119-
.add('Validation', () => {
120-
function isFakeValidator() {
121-
return false;
159+
.add('Validation 2 fieldsets', () => {
160+
const isCats = value => ({
161+
isCats: value.input1 === 'cats',
162+
});
163+
localize.locale = 'en-GB';
164+
try {
165+
localize.addData('en-GB', 'lion-validate+isCats', {
166+
error: {
167+
isCats: '[Fieldset Nr. 1 Error] Input 1 needs to be "cats"',
168+
},
169+
});
170+
} catch (error) {
171+
// expected as it's a demo
122172
}
123173

124-
const fakeValidator = (...factoryParams) => [
125-
(...params) => ({ validator: isFakeValidator(...params) }),
126-
...factoryParams,
127-
];
128-
174+
const isDogs = value => ({
175+
isDogs: value.input1 === 'dogs',
176+
});
177+
localize.locale = 'en-GB';
129178
try {
130-
localize.addData('en-GB', 'lion-validate+validator', {
179+
localize.addData('en-GB', 'lion-validate+isDogs', {
131180
error: {
132-
validator: 'Fake error message',
181+
isDogs: '[Fieldset Nr. 2 Error] Input 1 needs to be "dogs"',
133182
},
134183
});
135184
} catch (error) {
136185
// expected as it's a demo
137186
}
138187

139188
return html`
140-
<lion-fieldset id="someId" .errorValidators=${[fakeValidator()]}>
141-
<lion-input name="input1" label="Label"></lion-input>
142-
<button
143-
@click=${() => {
144-
document.getElementById('someId').serializeGroup();
145-
}}
146-
>
147-
Submit
148-
</button>
189+
<lion-fieldset .errorValidators=${[[isCats]]}>
190+
<label slot="label">Fieldset Nr. 1</label>
191+
<lion-input
192+
label="An all time YouTube favorite"
193+
name="input1"
194+
help-text="longer then 2 characters"
195+
.errorValidators=${[minLengthValidator(3)]}
196+
></lion-input>
149197
</lion-fieldset>
150-
151198
<br />
199+
<hr />
152200
<br />
153-
<button>
154-
Tab-able
155-
</button>
201+
<lion-fieldset .errorValidators=${[[isDogs]]}>
202+
<label slot="label">Fieldset Nr. 2</label>
203+
<lion-input
204+
label="An all time YouTube favorite"
205+
name="input1"
206+
help-text="longer then 2 characters"
207+
.errorValidators=${[minLengthValidator(3)]}
208+
></lion-input>
209+
</lion-fieldset>
156210
`;
157211
});

‎packages/fieldset/test/lion-fieldset.test.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,17 +421,30 @@ describe('<lion-fieldset>', () => {
421421
<${childTag} name="input2"></${childTag}>
422422
</${tag}>
423423
`);
424+
const el2 = await fixture(html`
425+
<${tag}>
426+
<${childTag} name="input1"></${childTag}>
427+
<${childTag} name="input2"></${childTag}>
428+
</${tag}>
429+
`);
430+
424431
await nextFrame();
425432
const outside = await fixture(html`
426433
<button>outside</button>
427434
`);
428435

436+
outside.click();
437+
expect(el.touched, 'unfocused fieldset should stays untouched').to.be.false;
438+
429439
el.children[1].focus();
430440
el.children[2].focus();
431441
expect(el.touched).to.be.false;
432442

433443
outside.click(); // blur the group via a click
444+
outside.focus(); // a real mouse click moves focus as well
434445
expect(el.touched).to.be.true;
446+
447+
expect(el2.touched).to.be.false;
435448
});
436449

437450
it('potentially shows fieldset error message on interaction change', async () => {

0 commit comments

Comments
 (0)