fix(base,reactive-controllers): add delegates API and downstate delegate #4229

TarunAdobe · 2024-04-01T07:35:33Z

Description

This PR adds the delegate strategy in SWC that allows us to add downstate functionality over our existing components. This is needed so that we can support features like downstate in Spectrum 2 while keeping everything as it is in Spectrum 1

Downstate Activation is added to this PR

Motivation and context

The downstate feature in spectrum-two enhances user interaction by providing visual feedback when components are clicked or activated. It improves accessibility and user experience by clearly indicating the active state of elements. Developers can enable downstate for components, and it seamlessly integrates with existing functionality in the framework. This addition enriches the user interface, making interactions more intuitive and engaging.

Technical Document

How has this been tested?

Screenshots (if appropriate)

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
If my change required a change to the documentation, I have updated the documentation in this pull request.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

…bute

… into jnjosh/theming-systems

* fix(slider): no change in background for variant filled and fill start * chore(slider): updated readme for slider with fill start and variant filled --------- Co-authored-by: Rajdeep Chandra <rajdeepchandra@Rajdeeps-MacBook-Pro-2.local>

Bumps [next](https://github.com/vercel/next.js) from 14.1.0 to 14.1.4. - [Release notes](https://github.com/vercel/next.js/releases) - [Changelog](https://github.com/vercel/next.js/blob/canary/release.js) - [Commits](vercel/next.js@v14.1.0...v14.1.4) --- updated-dependencies: - dependency-name: next dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…bute

…ctrum-web-components into jnjosh/theming-systems

packages/button/src/ButtonBase.ts

tools/base/src/Base.ts

tools/reactive-controllers/src/SpectrumDelegates.ts

Westbrook

Not so sure why there's churn in the lock. I've seen it in other branches lately, so it might be related to renovate bot, or this might not be fully up to date with main?

Westbrook · 2024-05-03T12:44:13Z

tools/reactive-controllers/test/downstate.test.ts

+        );
+        await nextFrame();
+        await nextFrame();
+        controller.handlePointerDown();


We should use sendMouse to trigger this, rather than address the method directly.

Sure! the directly call is removed with a sendMouse trigger to mimic the call!

Westbrook · 2024-05-03T12:44:50Z

tools/reactive-controllers/test/downstate.test.ts

+import { html, LitElement } from 'lit';
+import { DownState } from '@spectrum-web-components/reactive-controllers/src/downstate.js';
+
+class TestDownstateEl extends LitElement {}


Can we use spectrumConfig here to ensure both the Controller and the delegates approach?

May also want to wrap it all in an <sp-theme> to get ALL of the contracts covered.

class TestDownstateEl extends SpectrumElement { override spectrumConfig = { downstate: ['spectrum-two'], }; protected override render(): TemplateResult { return html` <sp-theme system="spectrum-two" color="light" scale="medium"> <slot></slot> </sp-theme> `; } }

Westbrook · 2024-05-03T12:45:26Z

tools/reactive-controllers/src/downstate.ts

+        this.host?.removeEventListener('pointerdown', this.handlePointerDown);
+        this.host?.removeEventListener('pointerup', this.handlePointerUp);


We should remove the listeners with the abort signal.

removeEventListeners(): void { this.abortController.abort(); this.host?.removeEventListener('pointerdown', this.handlePointerDown); this.host?.removeEventListener('pointerup', this.handlePointerUp); }

The use of the abort controller here should prevent the need for removeEventListener code.

Westbrook · 2024-05-03T12:47:24Z

tools/reactive-controllers/src/downstate.ts

+    public handlePointerDown = (): void => {
+        this.updateDownStateStyles(true);
+    };
+
+    public handlePointerUp = (): void => {
+        this.updateDownStateStyles(false);
+    };


This applies these changes synchronously, which is probably correct. However, the majority of our render reactions are async, due to the reactivity of LitElement. Do you se any issue with this?

I feel updating the down state styles is a UI-specific operation and not directly related to LitElement's reactivity, handling it synchronously shouldn't cause any problems, but would love to hear your thoughts why you foresee some issues here?

Less about issues than about normalization and expectations. It's probably of no matter other than how it should be explicitly tested in https://github.com/adobe/spectrum-web-components/pull/4229/files/e9ea73081b0593fd29df23b1c67b00de68700cfb#r1589155295

This has been updated!

tools/reactive-controllers/test/downstate.test.ts

tools/reactive-controllers/src/downstate.ts

…-components into delegate-controllers

jnjosh and others added 19 commits March 15, 2024 11:51

refactor(theme): adds new system attribute and deprecates theme attri…

2983a36

…bute

chore(theme): made changes on theme attribute

956106f

chore: reverted THEME Key to support migrations

407efbf

feat: added system based delegates strategy

3fd8d2d

Merge branch 'main' of https://github.com/adobe/spectrum-web-components…

f7c2486

… into jnjosh/theming-systems

chore(menu): support delivering submenus via directives (#4226)

e7f42c8

Merge branch 'spectrum-two' into delegate-controllers

2ff9e5e

chore: theme registrated hard break

664d2c2

refactor(theme): adds new system attribute and deprecates theme attri…

9714b15

…bute

chore(theme): made changes on theme attribute

4830eed

chore: reverted THEME Key to support migrations

e69bb48

chore: theme registrated hard break

dc00e12

Merge branch 'jnjosh/theming-systems' of https://github.com/adobe/spe…

a20edc2

…ctrum-web-components into jnjosh/theming-systems

chore(theme): updated theme property to system

e159afc

chore(theme): updated theme to system in sp-theme in layout

576b603

chore(theme): updated config for VRT

4ffc9fb

chore(theme): updated config for VRT from theme to system

bfa82d9

Rajdeepc changed the title ~~Delegate controllers~~ feat(): added downstate delegate Apr 1, 2024

Rajdeepc changed the title ~~feat(): added downstate delegate~~ feat(): added downstate delegate for S2 Apr 1, 2024

Rajdeepc added the S2 label Apr 1, 2024

Rajdeepc requested changes Apr 2, 2024

View reviewed changes

packages/button/src/ButtonBase.ts Outdated Show resolved Hide resolved

tools/base/src/Base.ts Outdated Show resolved Hide resolved

tools/reactive-controllers/src/SpectrumDelegates.ts Outdated Show resolved Hide resolved

Rajdeepc requested a review from Westbrook April 2, 2024 10:56

Rajdeepc assigned TarunAdobe Apr 2, 2024

Rajdeep Chandra and others added 5 commits April 2, 2024 16:43

chore: update theme property to system

20595ee

fix: added spectrum system variant

6fe7a68

chore: updated theme for deprecation against system in theme files

2490dd5

chore: updated config to support system

8830483

feat: process s2 tokens and add s2 theme

8383a9d

Rajdeepc force-pushed the delegate-controllers branch from ca038ec to 12230c7 Compare May 2, 2024 11:19

Rajdeep Chandra and others added 9 commits May 2, 2024 16:56

chore: merge to swc-s2-styles

d19348c

chore: updated global vars

fc25f4d

chore: updated golden image cache

b3d6181

chore: removed global vars

19807c0

chore: removed lightest and darkest theme for spectrum-two

4cfc468

chore: fixed the ci config

0dc716a

chore: merged main

4c5799f

chore: updated golden image hash

49d9f9d

chore: merge to swc-s2-styles

e9ea730

Rajdeepc requested a review from Westbrook May 3, 2024 12:40

Westbrook reviewed May 3, 2024

View reviewed changes

TarunAdobe and others added 9 commits May 3, 2024 18:40

chore: fixed broken sidenav for vrts site

f704042

Merge branch 'main' into swc-s2-styles

1d004c6

chore: updated tests

6e339fe

Merge branch 'swc-s2-styles' of https://github.com/adobe/spectrum-web…

db9b11c

…-components into delegate-controllers

chore: added devmode warning for S2 theme

a2f1ac1

Merge branch 'main' into swc-s2-styles

3dee34b

chore: show devmode warning on using s2 theme only in devmode

d98d1be

chore: added devmode tests for code coverage

a32f837

chore: updated downstate abort controller

4a215da

Rajdeepc requested review from Westbrook, jnjosh and Rajdeepc and removed request for blunteshwar and Rajdeepc May 8, 2024 07:55

Merge branch 'swc-s2-styles' into delegate-controllers

d82b888

Rajdeepc force-pushed the swc-s2-styles branch 2 times, most recently from 6ca339b to 33c5c24 Compare May 13, 2024 12:28

Base automatically changed from swc-s2-styles to main May 14, 2024 07:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(base,reactive-controllers): add delegates API and downstate delegate #4229

fix(base,reactive-controllers): add delegates API and downstate delegate #4229

TarunAdobe commented Apr 1, 2024 •

edited by Rajdeepc

Westbrook left a comment

Westbrook May 3, 2024

Rajdeepc May 6, 2024

Westbrook May 3, 2024

Westbrook May 3, 2024

Rajdeepc May 6, 2024

Westbrook May 3, 2024

Rajdeepc May 6, 2024

Westbrook May 6, 2024

Westbrook May 3, 2024

Rajdeepc May 3, 2024

Westbrook May 3, 2024

Rajdeepc May 6, 2024

		this.host?.removeEventListener('pointerdown', this.handlePointerDown);
		this.host?.removeEventListener('pointerup', this.handlePointerUp);

fix(base,reactive-controllers): add delegates API and downstate delegate #4229

Are you sure you want to change the base?

fix(base,reactive-controllers): add delegates API and downstate delegate #4229

Conversation

TarunAdobe commented Apr 1, 2024 • edited by Rajdeepc

Description

Motivation and context

How has this been tested?

Screenshots (if appropriate)

Types of changes

Checklist

Best practices

Westbrook left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TarunAdobe commented Apr 1, 2024 •

edited by Rajdeepc