fix(label): prevent focusing and toggling elements slotted within a label when a text selection occurs by driskull · Pull Request #9990 · Esri/calcite-design-system

driskull · 2024-08-07T22:14:40Z

Related Issue: #8867

Summary

prevents internal label selection event from being emitted if a range of text selection is active
add e2e test

Notes

https://developer.mozilla.org/en-US/docs/Web/API/Window/getSelection
https://developer.mozilla.org/en-US/docs/Web/API/Selection/addRange

…abel when a text selection occurs. #8867

jcfranco

Looking great! Just had a note to update assertions to avoid testing the implementation detail.

jcfranco · 2024-08-12T18:44:42Z

+
+    const element = await page.find("calcite-label");
+
+    const eventSpy = await element.spyOnEvent(labelClickEvent);


Can you assert on the selection and unfocused input instead of the internal event (implementation detail)?

jcfranco

😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎
😎😎🆒🆒🆒😎😎🆒🆒😎😎😎🆒🆒😎😎🆒😎😎😎🆒😎
😎🆒😎😎😎😎🆒😎😎🆒😎🆒😎😎🆒😎🆒😎😎😎🆒😎
😎🆒😎😎😎😎🆒😎😎🆒😎🆒😎😎🆒😎🆒😎😎😎🆒😎
😎🆒😎😎😎😎🆒😎😎🆒😎🆒😎😎🆒😎🆒😎😎😎😎😎
😎😎🆒🆒🆒😎😎🆒🆒😎😎😎🆒🆒😎😎🆒🆒🆒😎🆒😎
😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎

jcfranco · 2024-08-12T19:06:12Z

+  it("should focus on the slotted form element when a label is clicked", async () => {
+    const page = await newE2EPage();
+    await page.setContent(`
+    <calcite-label id="do-not-duplicate-me" layout="inline-space-between">


Sorry for not spotting this earlier, but let's remove anything unrelated to the test. This id comes from a test specific to that prop/attr.

jcfranco · 2024-08-12T19:06:57Z

+  it("should not focus on the slotted form element when a label's text is selected", async () => {
+    const page = await newE2EPage();
+    await page.setContent(`
+    <calcite-label id="do-not-duplicate-me" layout="inline-space-between">


Bonus points if you bring in the html formatting helper. ✨🏆✨

jcfranco · 2024-08-12T19:11:26Z

    expect(element).toEqualAttribute("layout", "inline-space-between");
  });

+  it("should focus on the slotted form element when a label is clicked", async () => {


I like how this provides context for the selection test, but it is already covered by the labelable helper.

Maybe it's OK to merge with the selection test where it does the following

text is selected + click > no focus shift

text is unselected + click > focus shifts

?

I can remove this test since its testing duplicate functionality.

* origin/dev: fix(label): prevent focusing and toggling elements slotted within a label when a text selection occurs (#9990)

…to-monorepo * origin/dev: (37 commits) chore: release next fix(tree,tree-item): replace checkbox with div and a11y attributes (#9849) feat(combobox-item): add `heading` property (deprecates `textLabel`) and add `label` property (#9987) chore: stop requesting review from ben/franco for all PRs (#10040) feat: add close-caption, transcript, flag, and flag-slash (#10039) build(deps): update dependency @cspell/eslint-plugin to v8.13.2 (#10036) build(deps): update angular-cli monorepo to v18.1.4 (#10035) build(deps): clean up ui-icon dependencies (#10024) build: update browserslist db (#10033) docs: update component READMEs (#10034) ci: fix chromatic and pr-semantic check trigger events (#10041) chore: release next fix(label): prevent focusing and toggling elements slotted within a label when a text selection occurs (#9990) build(deps): update dependency @floating-ui/dom to v1.6.9 (#10025) build(deps): update dependency postcss to v8.4.41 (#10027) ci: new version of husky no longer need npx (#10030) build(deps): update dependency lerna to v8.1.8 (#10026) build(deps): update dependency chromatic to v11.7.0 (#9995) build(deps): update dependency eslint-plugin-jsdoc to v48.11.0 (#9997) chore: release next ...

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-ui-icons: 3.31.0</summary> ## [3.31.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-ui-icons@3.30.0...@esri/calcite-ui-icons@3.31.0) (2024-08-28) ### Features * Add close-caption, transcript, flag, and flag-slash ([#10039](#10039)) ([85c7d69](85c7d69)) * Add layer-connection ([#10067](#10067)) ([46dd2ee](46dd2ee)) ### Bug Fixes * Update close-caption, and transcript ([#10077](#10077)) ([2e86904](2e86904)) </details> <details><summary>@esri/calcite-components: 2.12.0</summary> ## [2.12.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@2.11.1...@esri/calcite-components@2.12.0) (2024-08-28) ### Features * **accordion, accordion-item:** Add component tokens ([#9861](#9861)) ([48f7f08](48f7f08)) * **action, action-bar, action-group, action-menu, action-pad:** Add component tokens ([#10058](#10058)) ([de96c24](de96c24)) * **alert:** Add 'queue' property to prioritize the ordering of alerts when opened ([#10029](#10029)) ([3aa47a4](3aa47a4)) * **combobox-item:** Add `heading` property (deprecates `textLabel`) and add `label` property ([#9987](#9987)) ([90920a0](90920a0)) * **dialog:** Add CSS variables to offset the position ([#9904](#9904)) ([660f0c5](660f0c5)) * **dialog:** Add escapeDisabled property ([#10081](#10081)) ([1ca8fba](1ca8fba)) * **dialog:** Add outsideCloseDisabled property ([#9978](#9978)) ([f1d89b9](f1d89b9)) * **dialog:** Add resizable and dragEnabled properties ([#9804](#9804)) ([e3e499e](e3e499e)) * **icon:** Add component tokens ([#10062](#10062)) ([34d5689](34d5689)) * **input-time-zone:** Display selected time zone country in region mode selection ([#9988](#9988)) ([1575729](1575729)) * **loader:** Add `determinate-value` type ([#9957](#9957)) ([7f21726](7f21726)) * **panel:** Add '--calcite-panel-content-space' style ([#10117](#10117)) ([0e3ebcb](0e3ebcb)) * **table:** Update multiple selection iconography ([#10125](#10125)) ([f2b2016](f2b2016)) * **tooltip:** Allow focusing and clicking on interactive elements within a tooltip ([#9914](#9914)) ([a3c144c](a3c144c)) ### Bug Fixes * **accordion-item:** Align focus styles across browsers ([#9944](#9944)) ([2e29b42](2e29b42)) * **alert:** Properly form a queue of alerts ([#10032](#10032)) ([b24d6ac](b24d6ac)) * **color-picker:** Prevent text selection when using color sliders/field ([#10043](#10043)) ([ab33e4d](ab33e4d)) * **combobox, input-date-picker, input-number, input-text, input-time-picker, input, radio-button-group, segmented-control, select, text-area:** Use toAriaBoolean in aria-invalid attribute to provide valid AT error messaging ([#10079](#10079)) ([78bc096](78bc096)) * **combobox:** Update placeholder-icon color ([#9956](#9956)) ([d916ca4](d916ca4)) * **dialog, flow-item:** Slotted closable panels should not close the component ([#10130](#10130)) ([ce2513d](ce2513d)) * **dialog:** Allow shell to be slotted inside ([#10118](#10118)) ([ea3a6de](ea3a6de)) * **dialog:** Fix escapeDisabled when the escape key is pressed on the panel ([#10097](#10097)) ([9a5f1f1](9a5f1f1)) * Ensure `beforeOpen`/`open` and `beforeClose`/`close` events emit properly ([#9958](#9958)) ([7e2764f](7e2764f)) * **filter:** Fix setFocus method ([#10149](#10149)) ([f4a959a](f4a959a)) * **input-date-picker:** Fix selection for the "bs" and "it-CH" locales ([#9976](#9976)) ([368d9e2](368d9e2)) * **input-date-picker:** Prevent console error when using a lang ([#10162](#10162)) ([e7bf604](e7bf604)) * **input-time-zone:** Translate region names ([#9940](#9940)) ([f975adb](f975adb)) * **input, combobox, input-date-picker, input-number, input-text, input-time-picker, radio-button-group, segmented-control, select, text-area:** Provide clear field error messaging for AT ([#9880](#9880)) ([46ad7d2](46ad7d2)) * **input, input-number, input-text:** No longer set focus when a validation message is clicked ([#10008](#10008)) ([529bb5a](529bb5a)) * **label:** Prevent focusing and toggling elements slotted within a label when a text selection occurs ([#9990](#9990)) ([49d6ae1](49d6ae1)) * **list-item:** Prevent scrolling when item is focused. ([#9948](#9948)) ([eab1e06](eab1e06)) * **panel:** Define heading and description line height ([#10085](#10085)) ([5478eb9](5478eb9)) * **slider:** Prevent text selection when using the slider ([#10073](#10073)) ([04bf325](04bf325)) * **slider:** Resolve step & snap floating point precision ([#10148](#10148)) ([613bc47](613bc47)) * **stepper-item:** Update component to take full width when parent's layout is "vertical" ([#10009](#10009)) ([03a5a30](03a5a30)) * **tree,tree-item:** Replace checkbox with div and a11y attributes ([#9849](#9849)) ([b1ac951](b1ac951)) ### Performance Improvements * **input-time-zone:** Improve memory footprint by leveraging browser cache and releasing utils after their use ([#9945](#9945)) ([a3d2141](a3d2141)) ### Reverts * "build(deps): remove unused cheerio package ([#10049](#10049))" ([#10057](#10057)) ([d271ab1](d271ab1)) * Add stories ([#10131](#10131)) ([e3a6590](e3a6590)) ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-ui-icons bumped from 3.31.0-next.4 to 3.31.0 </details> <details><summary>@esri/calcite-components-angular: 2.12.0</summary> ## [2.12.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-angular@2.11.1...@esri/calcite-components-angular@2.12.0) (2024-08-28) ### Miscellaneous Chores * **@esri/calcite-components-angular:** Synchronize components versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0-next.34 to 2.12.0 </details> <details><summary>@esri/calcite-components-react: 2.12.0</summary> ## [2.12.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@2.11.1...@esri/calcite-components-react@2.12.0) (2024-08-28) ### Miscellaneous Chores * **@esri/calcite-components-react:** Synchronize components versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0-next.34 to 2.12.0 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-ui-icons: 3.31.0</summary> ## [3.31.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-ui-icons@3.30.0...@esri/calcite-ui-icons@3.31.0) (2024-08-28) ### Features * Add close-caption, transcript, flag, and flag-slash ([#10039](#10039)) ([85c7d69](85c7d69)) * Add layer-connection ([#10067](#10067)) ([46dd2ee](46dd2ee)) ### Bug Fixes * Update close-caption, and transcript ([#10077](#10077)) ([2e86904](2e86904)) </details> <details><summary>@esri/calcite-components: 2.12.0</summary> ## [2.12.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@2.11.1...@esri/calcite-components@2.12.0) (2024-08-28) ### Features * **accordion, accordion-item:** Add component tokens ([#9861](#9861)) ([48f7f08](48f7f08)) * **action, action-bar, action-group, action-menu, action-pad:** Add component tokens ([#10058](#10058)) ([de96c24](de96c24)) * **alert:** Add 'queue' property to prioritize the ordering of alerts when opened ([#10029](#10029)) ([3aa47a4](3aa47a4)) * **combobox-item:** Add `heading` property (deprecates `textLabel`) and add `label` property ([#9987](#9987)) ([90920a0](90920a0)) * **dialog:** Add CSS variables to offset the position ([#9904](#9904)) ([660f0c5](660f0c5)) * **dialog:** Add escapeDisabled property ([#10081](#10081)) ([1ca8fba](1ca8fba)) * **dialog:** Add outsideCloseDisabled property ([#9978](#9978)) ([f1d89b9](f1d89b9)) * **dialog:** Add resizable and dragEnabled properties ([#9804](#9804)) ([e3e499e](e3e499e)) * **icon:** Add component tokens ([#10062](#10062)) ([34d5689](34d5689)) * **input-time-zone:** Display selected time zone country in region mode selection ([#9988](#9988)) ([1575729](1575729)) * **loader:** Add `determinate-value` type ([#9957](#9957)) ([7f21726](7f21726)) * **panel:** Add '--calcite-panel-content-space' style ([#10117](#10117)) ([0e3ebcb](0e3ebcb)) * **table:** Update multiple selection iconography ([#10125](#10125)) ([f2b2016](f2b2016)) * **tooltip:** Allow focusing and clicking on interactive elements within a tooltip ([#9914](#9914)) ([a3c144c](a3c144c)) ### Bug Fixes * **accordion-item:** Align focus styles across browsers ([#9944](#9944)) ([2e29b42](2e29b42)) * **alert:** Properly form a queue of alerts ([#10032](#10032)) ([b24d6ac](b24d6ac)) * **color-picker:** Prevent text selection when using color sliders/field ([#10043](#10043)) ([ab33e4d](ab33e4d)) * **combobox, input-date-picker, input-number, input-text, input-time-picker, input, radio-button-group, segmented-control, select, text-area:** Use toAriaBoolean in aria-invalid attribute to provide valid AT error messaging ([#10079](#10079)) ([78bc096](78bc096)) * **combobox:** Update placeholder-icon color ([#9956](#9956)) ([d916ca4](d916ca4)) * **dialog, flow-item:** Slotted closable panels should not close the component ([#10130](#10130)) ([ce2513d](ce2513d)) * **dialog:** Allow shell to be slotted inside ([#10118](#10118)) ([ea3a6de](ea3a6de)) * **dialog:** Fix escapeDisabled when the escape key is pressed on the panel ([#10097](#10097)) ([9a5f1f1](9a5f1f1)) * Ensure `beforeOpen`/`open` and `beforeClose`/`close` events emit properly ([#9958](#9958)) ([7e2764f](7e2764f)) * **filter:** Fix setFocus method ([#10149](#10149)) ([f4a959a](f4a959a)) * **input-date-picker:** Fix selection for the "bs" and "it-CH" locales ([#9976](#9976)) ([368d9e2](368d9e2)) * **input-date-picker:** Prevent console error when using a lang ([#10162](#10162)) ([e7bf604](e7bf604)) * **input-time-zone:** Translate region names ([#9940](#9940)) ([f975adb](f975adb)) * **input, combobox, input-date-picker, input-number, input-text, input-time-picker, radio-button-group, segmented-control, select, text-area:** Provide clear field error messaging for AT ([#9880](#9880)) ([46ad7d2](46ad7d2)) * **input, input-number, input-text:** No longer set focus when a validation message is clicked ([#10008](#10008)) ([529bb5a](529bb5a)) * **label:** Prevent focusing and toggling elements slotted within a label when a text selection occurs ([#9990](#9990)) ([49d6ae1](49d6ae1)) * **list-item:** Prevent scrolling when item is focused. ([#9948](#9948)) ([eab1e06](eab1e06)) * **panel:** Define heading and description line height ([#10085](#10085)) ([5478eb9](5478eb9)) * **slider:** Prevent text selection when using the slider ([#10073](#10073)) ([04bf325](04bf325)) * **slider:** Resolve step & snap floating point precision ([#10148](#10148)) ([613bc47](613bc47)) * **stepper-item:** Update component to take full width when parent's layout is "vertical" ([#10009](#10009)) ([03a5a30](03a5a30)) * **tree,tree-item:** Replace checkbox with div and a11y attributes ([#9849](#9849)) ([b1ac951](b1ac951)) ### Performance Improvements * **input-time-zone:** Improve memory footprint by leveraging browser cache and releasing utils after their use ([#9945](#9945)) ([a3d2141](a3d2141)) ### Reverts * "build(deps): remove unused cheerio package ([#10049](#10049))" ([#10057](#10057)) ([d271ab1](d271ab1)) * Add stories ([#10131](#10131)) ([e3a6590](e3a6590)) ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-ui-icons bumped from 3.31.0-next.4 to 3.31.0 </details> <details><summary>@esri/calcite-components-angular: 2.12.0</summary> ## [2.12.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-angular@2.11.1...@esri/calcite-components-angular@2.12.0) (2024-08-28) ### Miscellaneous Chores * **@esri/calcite-components-angular:** Synchronize components versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0-next.34 to 2.12.0 </details> <details><summary>@esri/calcite-components-react: 2.12.0</summary> ## [2.12.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@2.11.1...@esri/calcite-components-react@2.12.0) (2024-08-28) ### Miscellaneous Chores * **@esri/calcite-components-react:** Synchronize components versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0-next.34 to 2.12.0 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> **Related Issue:** # ## Summary Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

fix(label): prevent focusing and toggling elements slotted within a l…

b49fdb9

…abel when a text selection occurs. #8867

github-actions Bot assigned driskull Aug 7, 2024

github-actions Bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Aug 7, 2024

driskull added the skip visual snapshots Pull requests that do not need visual regression testing. label Aug 7, 2024

Merge branch 'dev' into dris0000/label-click-selection

1a2abbc

driskull marked this pull request as ready for review August 7, 2024 23:32

driskull requested a review from jcfranco August 7, 2024 23:32

jcfranco requested changes Aug 12, 2024

View reviewed changes

review fixes

9d10a8b

driskull requested a review from jcfranco August 12, 2024 19:03

jcfranco approved these changes Aug 12, 2024

View reviewed changes

review fixes

e79ba21

driskull merged commit 49d6ae1 into dev Aug 12, 2024

driskull deleted the dris0000/label-click-selection branch August 12, 2024 19:35

github-actions Bot added this to the 2024-08-27 - Aug Release milestone Aug 12, 2024

benelan added a commit that referenced this pull request Aug 12, 2024

Merge remote-tracking branch 'origin/dev' into benelan/icon-ci-fixes

595fd6c

* origin/dev: fix(label): prevent focusing and toggling elements slotted within a label when a text selection occurs (#9990)

benelan mentioned this pull request Aug 28, 2024

chore: release main #10171

Merged

benelan mentioned this pull request Aug 28, 2024

chore: release main (#10171) #10172

Merged

benelan mentioned this pull request Aug 28, 2024

chore: release main (#10171) #10173

Closed

calcite-admin mentioned this pull request Sep 30, 2024

chore: release main #10457

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(label): prevent focusing and toggling elements slotted within a label when a text selection occurs#9990

fix(label): prevent focusing and toggling elements slotted within a label when a text selection occurs#9990
driskull merged 4 commits intodevfrom
dris0000/label-click-selection

driskull commented Aug 7, 2024

Uh oh!

jcfranco left a comment

Uh oh!

jcfranco Aug 12, 2024

Uh oh!

jcfranco left a comment

Uh oh!

jcfranco Aug 12, 2024

Uh oh!

jcfranco Aug 12, 2024

Uh oh!

jcfranco Aug 12, 2024

Uh oh!

driskull Aug 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		const element = await page.find("calcite-label");

		const eventSpy = await element.spyOnEvent(labelClickEvent);

Conversation

driskull commented Aug 7, 2024

Summary

Notes

Uh oh!

jcfranco left a comment

Choose a reason for hiding this comment

Uh oh!

jcfranco Aug 12, 2024

Choose a reason for hiding this comment

Uh oh!

jcfranco left a comment

Choose a reason for hiding this comment

Uh oh!

jcfranco Aug 12, 2024

Choose a reason for hiding this comment

Uh oh!

jcfranco Aug 12, 2024

Choose a reason for hiding this comment

Uh oh!

jcfranco Aug 12, 2024

Choose a reason for hiding this comment

Uh oh!

driskull Aug 12, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants