upgrade: focus and active-element packages migration and adaptation upgrade for Solid 2.0#896
upgrade: focus and active-element packages migration and adaptation upgrade for Solid 2.0#896davedbase wants to merge 14 commits into
Conversation
🦋 Changeset detectedLatest commit: 92ed7f7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
# Conflicts: # pnpm-lock.yaml
# Conflicts: # packages/utils/src/index.ts # pnpm-lock.yaml
# Conflicts: # packages/utils/src/index.ts
# Conflicts: # packages/utils/src/index.ts # pnpm-lock.yaml
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/focus/src/focusSignal.ts`:
- Around line 45-48: The initial focus-state initializer passed to
createHydratableSignal wrongly compares document.activeElement to the reactive
accessor itself when target may be an accessor; update that initializer in the
createHydratableSignal call (the arrow function creating isActive/setIsActive)
to resolve the accessor first (e.g., const el = typeof target === 'function' ?
target() : target) and then compare document.activeElement === el so the initial
isActive is correct for both element targets and accessor targets.
In `@README.md`:
- Line 26: Update the README row for the focus package to list all migrated APIs
by adding makeFocusListener, createFocusSignal, and createFocusTrap alongside
autofocus and createAutofocus; locate the table row containing the focus package
badges and links (the line referencing autofocus/createAutofocus) and append
links and labels for makeFocusListener, createFocusSignal, and createFocusTrap
so the package entry reflects the current exported primitives.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7abc55b6-f96d-48c5-8e67-f69ab368afd8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
.changeset/active-element-solid2-migration.md.changeset/autofocus-solid2-migration.md.changeset/focus-solid2-migration.md.changeset/pre.jsonREADME.mdpackages/active-element/package.jsonpackages/active-element/src/index.tspackages/active-element/test/index.test.tspackages/active-element/test/server.test.tspackages/autofocus/README.mdpackages/autofocus/test/index.test.tsxpackages/focus/CHANGELOG.mdpackages/focus/LICENSEpackages/focus/README.mdpackages/focus/dev/index.tsxpackages/focus/package.jsonpackages/focus/src/autofocus.tspackages/focus/src/focusSignal.tspackages/focus/src/focusTrap.tspackages/focus/src/index.tspackages/focus/test/index.test.tsxpackages/focus/test/server.test.tspackages/focus/tsconfig.jsonpackages/utils/src/index.ts
💤 Files with no reviewable changes (3)
- .changeset/autofocus-solid2-migration.md
- packages/autofocus/README.md
- packages/autofocus/test/index.test.tsx
Summary
autofocuspackage to@solid-primitives/focus— directory, npm name, all internal references, and the root README table row updatedcreateFocusTrapfrom solid-focus-trap (corvu, by Jasmin Noetzli / GiyoMoon) into thefocuspackage, fully adapted for Solid.js 2.0afterPaint(double-requestAnimationFramehelper) to@solid-primitives/utils, ported from@corvu/utils/domautofocus/createAutofocusinto their ownsrc/autofocus.tsfile, consistent withsrc/focusTrap.tsLICENSE,package.jsoncontributors, andREADME.mdWhat
createFocusTrapdoesTraps keyboard focus inside a given DOM element, cycling through focusable children on Tab / Shift+Tab. Uses a
MutationObserverto react to DOM changes and restores focus to the previously-focused element when deactivated.Solid 2.0 adaptations
createEffectuses the split compute/apply model; cleanup returned from apply instead ofonCleanupmergePropsremoved — defaults handled inline with?? truecreateSignalforfocusableElementsusesINTERNAL_OPTIONS(ownedWrite: true) since it is written from within effect apply functionsaccess/MaybeAccessorsourced from@solid-primitives/utilsinstead of@corvu/utils/reactivityafterPaintsourced from@solid-primitives/utilsinstead of@corvu/utils/domBreaking Changes
Peer dependencies:
solid-js@^2.0.0-beta.12and@solidjs/web@^2.0.0-beta.12are now required.makeFocusListenerandcreateFocusSignalhave moved to@solid-primitives/focus. Import them from there instead:isServeris now sourced from@solidjs/webinternally (no user-facing API change)Summary by CodeRabbit
Release Notes
Breaking Changes
use:autofocusdirective has been removed; useref={autofocus()}as a ref callback instead.makeFocusListenerandcreateFocusSignalhave been moved to@solid-primitives/focuspackage.New Features
createFocusTrapfor keyboard focus management and trapping within containers.@solid-primitives/focuspackage.