Skip to content

Conversation

kosiew
Copy link
Contributor

@kosiew kosiew commented Jul 4, 2025

Which issue does this PR close?

Rationale for this change

This change simplifies the logic for injecting HTML styles in the DataFrameHtmlFormatter class. The previous implementation relied on a class-level _styles_loaded flag to prevent redundant style injection, which introduced state management complexity and testing challenges. By using a <script> block that conditionally appends styles based on DOM inspection, this new approach avoids side effects and supports better isolation across render calls and notebook sessions.

What changes are included in this PR?

  • Removed _styles_loaded flag and associated logic.
  • Replaced conditional style injection with DOM-based script that ensures styles are added once.
  • Removed the reset_styles_loaded_state function and its test usages.
  • Updated tests to validate the presence of injected script and consistent behavior across renders.
  • Ensured fallback CSS inclusion and custom styles are still respected.

Are these changes tested?

Yes, all existing relevant tests have been updated to reflect the new behavior. Assertions now check for presence of the injected script rather than <style> tags. This ensures that styles are being correctly applied through the new mechanism.

Are there any user-facing changes?

Yes, but minimal:

  • Users no longer need to manually reset styles between sessions using reset_styles_loaded_state.
  • HTML output now uses <script>-based style injection, which improves compatibility and reduces potential styling issues across multiple renderings.

@kosiew kosiew self-assigned this Jul 4, 2025
Copy link
Member

@timsaucer timsaucer left a comment

Choose a reason for hiding this comment

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

Thank you for this!

Also, I recently implemented custom formatter for some code in another project and it worked like a charm! These features are excellent.

@timsaucer timsaucer merged commit 2e1b713 into apache:main Jul 4, 2025
17 checks passed
@kosiew
Copy link
Contributor Author

kosiew commented Jul 5, 2025

You're welcome.

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.

Add DOM-guarded CSS/JS injection to DataFrameHtmlFormatter to prevent duplicate style/script inserts

2 participants