Skip to content

Store theme names in a node feature like ClassList instead of parsing the theme attribute #25603

Description

@totally-not-ai

Description

ThemeList is the only one of the three token-based element APIs that has no backing storage of its own. Element.getClassList() returns a view over the ElementClassList node feature and Element.getStyle() returns a view over ElementStylePropertyMap, but Element.getThemeList() returns a ThemeListImpl that parses the theme attribute string on every operation and writes the joined value back.

That works, and #25590 made it consistent by removing the cached copy of the names that used to go stale, but it leaves ThemeList structurally different from its two siblings:

  • Every read splits a string and every write joins one, instead of operating on a list.
  • iterator() cannot be a live view the way ClassList's is, because there is no list to iterate — it walks a snapshot of the parsed value, which is now documented as a deliberate exception.
  • A theme name cannot contain spaces, and this has to be validated on the way in, because the serialized form is the only storage. ClassList validates for the same reason, but there the restriction is inherent to the DOM rather than to Flow's storage choice.
  • The whole attribute value is re-sent to the client whenever a single theme name changes, rather than a single list operation.

Proposal

Store the theme names in a dedicated node feature and derive the attribute from it, exactly the way class already works:

  1. Add an ElementThemeList node feature extending SerializableNodeList<String>, mirroring ElementClassList, with a ThemeList set view over it.
  2. Register a ThemeAttributeHandler in CustomAttribute for theme, mirroring ClassAttributeHandler, so that getAttribute("theme") returns the concatenated names and setAttribute("theme", "...") replaces the list. This is the mechanism that already keeps class and getClassList() in agreement, so no new machinery is needed.
  3. Point Element.getThemeList() at the state provider (getStateProvider().getThemeList(getNode())) instead of constructing a ThemeListImpl, and retire ThemeListImpl.
  4. Move the signal binding support (bind(String, Signal) and bind(Signal<List<String>>)) onto the new view, reusing the ElementClassList implementation as the template — the two are near-identical today.

Why the next major

  • theme stops being a plain attribute in the state node, which changes the serialized form and anything that inspects it directly.
  • setAttribute("theme", ...) gains the same "overrides anything set previously via getThemeList()" semantics that setAttribute("class", ...) already documents.
  • ThemeListImpl is public (though marked for internal use only) and would be removed.

Notes

  • HasTheme.setThemeName(String) is documented as taking a space-separated string and currently writes the attribute directly; it would go through the custom attribute handler instead, which is the behaviour it already relies on for class.
  • IndexHtmlRequestHandler writes theme on the html element of the bootstrap document through jsoup, not through Element, so it is unaffected.
  • Once the names live in a list, the iterator can become a live view and the snapshot exception documented in Element.getThemeList() can be dropped.

Follow-up to #25590, which fixed the staleness within the current design. Split out from #25575.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions