Skip to content

fix(registry): keep equal Fortal themes from rebuilding token readers - #325

Open
leoafarias wants to merge 3 commits into
mainfrom
fix/registry-fortal-token-equality
Open

leoafarias wants to merge 3 commits into
mainfrom
fix/registry-fortal-token-equality

Conversation

@leoafarias

Copy link
Copy Markdown
Collaborator

Description

Fortal's shadow tokens are Lists, and a plain List equals only itself. MixScope decides whether to notify its dependents by comparing the old and new token maps value by value, so every FortalTheme rebuild, even with identical settings, notified every widget that reads a Fortal token. Vanilla's tokens are all value-equal and were not affected.

buildFortalScopeTokens now returns its 22 shadow lists (6 List<BoxShadow>, 16 List<RemixBoxShadow>) as unmodifiable lists that compare by value, so an equal theme builds an equal token map. The registry template and the demo and dashboard installed copies are regenerated.

The second commit documents that every theme scope (Vanilla and Fortal) installs a complete token set instead of merging with the scope above it, so a MixScope.inherit override made above a nested theme, such as a custom focusRingWidth, does not apply below it. That is existing behavior and matches Vanilla and the building-remix-design-system skill's scope contract; a plain merge would not change it for Fortal tokens because the nested map contains every Fortal key. No code change for it.

Validation

  • New theme_rebuild_test.dart counts builds of a token-reading descendant across an equal rebuild (light and dark) and a changed accent. It fails on main (Expected: <1> Actual: <2>) and passes here.
  • registry_source full suite 1392 passed; apps/demo 64 and apps/dashboard 84 passed.
  • dart format clean; dart analyze clean on registry_source and the two touched apps; ci:checks, docs:check, open-code:registry:build (no drift), and open-code:apps:check pass.

Related Issues

Follow-up to review of #319 (finding on the Fortal rebuild amplifier) and #320 (nested scope reset note).

Checklist

  • My PR includes unit or integration tests for all changed/updated/fixed behaviors.
  • I have updated or added relevant documentation (doc comments with ///).
  • I am prepared to follow up on review comments in a timely manner.

Breaking Change

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

Fortal's shadow tokens are lists, and a plain List equals only itself. MixScope
compares the old and new token maps value by value to decide whether to notify
its dependents, so every FortalTheme rebuild, even with identical settings,
notified every widget that reads a Fortal token. Vanilla's tokens are all
value-equal and were never affected.

buildFortalScopeTokens now returns its shadow lists as unmodifiable lists that
compare by value, so an equal theme builds an equal token map. The registry
template and the demo and dashboard installed copies are regenerated.
…des above it

Every theme scope installs a complete token set instead of merging with the
scope above it, in Vanilla and in Fortal alike, so a MixScope.inherit override
made above a nested theme does not apply below it. Say so next to the
description of nested scopes, and where to place the override instead.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T01:16:52.481217Z 328aaa2 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant