Skip to content

Conversation

@mwouts
Copy link
Owner

@mwouts mwouts commented Aug 23, 2025

Closes #424

@mwouts mwouts requested a review from Copilot August 23, 2025 14:40
@github-actions
Copy link

Thank you for making this pull request.

Did you know? You can try it on Binder: Binder:lab.

Also, the version of ITables developed in this PR can be installed with pip:

pip install git+https://github.com/mwouts/itables.git@robust_loading_of_offline_itables

(this requires nodejs, see more at Developing ITables)

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR makes ITables robust to HTML display order in offline mode by implementing event-driven initialization that works even when the initialization cell is rendered after the table cells. This addresses issues in VS Code where display order isn't guaranteed.

  • Refactored offline mode to use event-driven initialization with custom events
  • Split template files for cleaner separation between online and offline modes
  • Updated test expectations to match new error message format and file size changes

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/itables/javascript.py Refactored replace_value function and offline template generation with event-driven approach
src/itables/html/init_notebook_offline.html New template for offline initialization with event dispatching
src/itables/html/datatables_template_offline.html New offline-specific table template with event listening
tests/test_javascript.py Updated tests for improved replace_value function with expected_count parameter
tests/test_connected_notebook_is_small.py Adjusted file size expectations for offline notebooks
src/itables/version.py Bumped version to 2.5.0-dev
docs/changelog.md Added changelog entry for the fix

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 320 to +323
count = template.count(pattern)
if not count:
raise ValueError("pattern={} was not found in template".format(pattern))
elif count > 1:
if count != expected_count:
raise ValueError(
"pattern={} was found multiple times ({}) in template".format(
pattern, count
)
f"{pattern=} was found {count} times in template, expected {expected_count}."
Copy link

Copilot AI Aug 23, 2025

Choose a reason for hiding this comment

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

The error message should use consistent formatting. Either use f-string format for both pattern and count, or use traditional string formatting for consistency with the rest of the codebase.

Copilot uses AI. Check for mistakes.
@codecov-commenter
Copy link

codecov-commenter commented Aug 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.14%. Comparing base (1ad5284) to head (46a252d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #427      +/-   ##
==========================================
+ Coverage   88.11%   88.14%   +0.02%     
==========================================
  Files          50       50              
  Lines        2020     2025       +5     
==========================================
+ Hits         1780     1785       +5     
  Misses        240      240              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mwouts mwouts merged commit e63f81e into main Aug 28, 2025
15 checks passed
@mwouts mwouts deleted the robust_loading_of_offline_itables branch August 29, 2025 22:38
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.

connected=False does not work reliably in VS Code in v2.4.x

3 participants