Skip to content

fix(lit): accept partial hinted text styles#832

Open
stablegenius49 wants to merge 1 commit intogoogle:mainfrom
stablegenius49:pr-factory/issue-602-hinted-styles
Open

fix(lit): accept partial hinted text styles#832
stablegenius49 wants to merge 1 commit intogoogle:mainfrom
stablegenius49:pr-factory/issue-602-hinted-styles

Conversation

@stablegenius49
Copy link

Summary

  • stop the lit text renderer from requiring every hinted text style key before it recognizes additionalStyles.Text as hinted styles
  • remove the impossible h6 expectation and treat partial hinted style maps as hinted when they contain at least one recognized hinted key
  • add a regression test covering partial hinted styles vs flat style maps

Testing

  • cd renderers/web_core && npm run build
  • cd renderers/lit && npm test

Fixes #602

@google-cla
Copy link

google-cla bot commented Mar 12, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request fixes an issue where the lit text renderer required all hinted text style keys to be present, and now correctly accepts partial hinted text styles. The logic is updated to check for at least one valid key, and the related types and functions have been refactored into a new file. A regression test has also been added, which is great. The changes are solid, but I've left one comment regarding a mismatch in a type definition that could be improved for better type safety.

Comment on lines +17 to +25
export interface HintedStyles {
h1: Record<string, string>;
h2: Record<string, string>;
h3: Record<string, string>;
h4: Record<string, string>;
h5: Record<string, string>;
body: Record<string, string>;
caption: Record<string, string>;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The isHintedStyles type guard correctly validates partially defined hinted styles, but the HintedStyles interface requires all properties to be present. This creates a mismatch where the type system will incorrectly assume all properties exist after the type guard passes. To make the type definition accurate and prevent potential runtime errors, all properties on HintedStyles should be optional.

Suggested change
export interface HintedStyles {
h1: Record<string, string>;
h2: Record<string, string>;
h3: Record<string, string>;
h4: Record<string, string>;
h5: Record<string, string>;
body: Record<string, string>;
caption: Record<string, string>;
}
export interface HintedStyles {
h1?: Record<string, string>;
h2?: Record<string, string>;
h3?: Record<string, string>;
h4?: Record<string, string>;
h5?: Record<string, string>;
body?: Record<string, string>;
caption?: Record<string, string>;
}

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

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Lit renderer - hintedStyles assumption causes additionalStyles to be ignored

2 participants