feat(shared)!: add ElementInternalsMixin and @cssState decorator - #3612
Draft
jpzwarte wants to merge 3 commits into
Draft
feat(shared)!: add ElementInternalsMixin and @cssState decorator#3612jpzwarte wants to merge 3 commits into
jpzwarte wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: 1bf3b50 The changes in this PR will be included in the next version bump. This PR includes changesets to release 45 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an
ElementInternalsMixinand a@cssStatedecorator to@sl-design-system/shared, and usesthem across the components that work with
ElementInternalsand custom CSS states. Closes #3610.ElementInternalsMixinattaches theElementInternalsand exposes them aselementInternals,so the component, its subclasses, other mixins and its tests all have one standardized way of
getting to them. It replaces the
readonly internals = this.attachInternals()line every formassociated element had. A mixin instead of a decorator, because a decorator cannot add a member to
the type of the class in either the legacy or the standard mode; a mixin can.
@cssStatekeeps a custom CSS state in sync with a boolean property or getter, replacing thehand written
if (changes.has('x')) { states.add/delete }blocks inupdated(). The state namedefaults to the dasherized property name. It works with both the legacy
(
experimentalDecorators) and the standard TC39 decorators, and uses a single controller perelement no matter how many properties it decorates.
Both are exported explicitly, as
@sl-design-system/shared/mixins/element-internals.jsand@sl-design-system/shared/decorators/css-state.js, not through the barrels.Breaking change
A Form Associated Custom Element now has to expose its
ElementInternalsaselementInternalsinstead of
internals, since that is whatFormControlMixinreads the validity and form valuefrom. All the components in this repo have been updated; the
internalsproperty is gone from them.Refactored
ElementInternalsMixinis applied to 14 components:button,button-bar,checkbox-group,combobox,date-field,editor,radio-group,select,select-button,switch,tag,time-field,toggle-buttonandtool-bar, plus thechild-formexample. It wrapsLitElementdirectly, so the mixins that use the internals sit above the one that provides them.
@cssStatereplaces the manual state juggling for 8 states in 4 components:select-buttonclearable,clear-focused,has-selectiontoggle-buttonpressed,icon-only,text-onlydate-fieldplaceholder-shown,has-valuetime-fieldplaceholder-shown,has-valueThe
toggle-buttonconversion also removes a workaround:render()reads theicon-onlystate, soupdated()compared the old and new value to trigger an extrarequestUpdate().@cssStateapplies in
hostUpdate(), before render, so that extra render pass is gone.States that are computed from measurements or set from event handlers are left as they are
(
tag,tool-bar,overflow,button,button-bar, andhas-focusin the date and time field).Converting those means giving them a backing property and deferring them to the next update, which
is a behaviour change rather than a refactor.
Notes
fix/3415-disabled-switch-tooltip-v2(feat(switch): refactor switch #3604) instead ofmain, so the diff shows onlythis work. Retarget to
mainonce that one is merged.@cssPropertydecorator is described in [Shared] Investigate if a@cssPropertydecorator is beneficial #3611.