Skip to content

Move the CSS imports to .css files #388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 6, 2025
Merged

Move the CSS imports to .css files #388

merged 3 commits into from
Jun 6, 2025

Conversation

mwouts
Copy link
Owner

@mwouts mwouts commented May 30, 2025

This PR implements a change suggested by @mscolnick in this comment.

While the change makes complete sense, and does set the ._css attribute on the Jupyter widget as expected, it does not yet fix the issues seen in Marimo with v2.4.0, that is #383 and #387 .

Myles, sorry for asking, but the fact is that I know too little about CSS and shadow dom. Do you see what else I need to change on the ._css field to make this work? In this simple example I would expect selected rows to appear in blue, but they don't:
image

Notes to myself:

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@css_in_marimo

(this requires nodejs, see more at Developing ITables)

@codecov-commenter
Copy link

codecov-commenter commented May 30, 2025

Codecov Report

Attention: Patch coverage is 63.43284% with 49 lines in your changes missing coverage. Please review.

Project coverage is 87.85%. Comparing base (1a65c2e) to head (cf0177c).

Files with missing lines Patch % Lines
packages/dt_for_itables/add_host_to_root.py 60.00% 20 Missing ⚠️
apps/marimo/column_visibility.py 45.45% 12 Missing ⚠️
apps/marimo/selected_rows.py 50.00% 10 Missing ⚠️
apps/marimo/search_panes.py 53.33% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #388      +/-   ##
==========================================
- Coverage   89.62%   87.85%   -1.77%     
==========================================
  Files          44       49       +5     
  Lines        1851     1985     +134     
==========================================
+ Hits         1659     1744      +85     
- Misses        192      241      +49     

☔ 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.

@mscolnick
Copy link

Hey @mwouts ill check out your branch on Sunday or Monday and check it out. Whats the fastest way to run from source once I check out your branch?

@mwouts
Copy link
Owner Author

mwouts commented May 31, 2025

Hey @mwouts ill check out your branch on Sunday or Monday and check it out. What's the fastest way to run from source once I check out your branch?

Oh thank you, much appreciated!

To run from source you can simply install ITables in development mode with

pip install -e .

You will need nodejs in your environment (if necessary you can create a new env with conda mamba create --file environment.yml)

The installation of ITables in development mode takes a while (1-2 miutes), that's because it builds the four node modules (main package dt_for_itables, the widget one itables_anywidget, plus the dash and the streamlit component). And it does not really provides a JS development mode - if you edit any of the node modules you will have to either re-install, or (much quicker), to re-build the packages you changed, like this:

cd packages/dt_for_itables
npm run build
cd ../..
cd packages/itables_anywidget
npm run build
cd ../..

@mscolnick
Copy link

@mwouts i just played around with it locally.

it looks to be from :root in the css. if you can change it to :root, :host {. That should fix it (at least part of it).

@mwouts
Copy link
Owner Author

mwouts commented Jun 2, 2025

Thank you very much @mscolnick , that does fix the selected row issue indeed (#383) - see this updated screenshot:

image

May I ask you two more questions?

  1. Is there a "clean" way to replace :root with :root, :host in the CSS (currently generated by @import 'dt_for_itables/dt_bundle.css'; at packages/itables_anywidget/js/widget.css) - other than a text search and replace?
  2. The trick does not seem to address the "column visibility" button issue (Paging, column visibility selectors invisible in ITable (2.4.0, marimo/anywidget) #387), do you have any idea what could help with that one?

@mscolnick
Copy link

  1. I'd probably do find/replace. Not sure where the root is coming from, but if you own the source, could change it there.

@mscolnick
Copy link

  1. I wasn't sure what this issue is. Mind explaining it a bit more to me with some screenshots

@mwouts
Copy link
Owner Author

mwouts commented Jun 2, 2025

  1. I'd probably do find/replace. Not sure where the root is coming from, but if you own the source, could change it there.

Yes a find and replace might be the simplest way! The CSS comes from datatables.net and is owned by Allan Jardine, not by myself.

  1. I wasn't sure what this issue is. Mind explaining it a bit more to me with some screenshots
    In Jupyter, when we click on the lengthMenu or on the column visibility button, there is another interface coming up like here - but we don't see that in Marimo yet:
    image

Let me also ping @AllanJard, the author of DataTables. Allan, we are trying to get ITables to work in the context of the (Python) Marimo notebooks. We're almost there, but Marimo has the specificity of using shadow dom, so we had to make a small change (see above) to get the CSS to work and highlight the selected rows. Now we are left with a last issue, which is that the colvis and page length buttons don't work yet (clicking on the button does nothing, unlike expected above). Allan, would you have some advice about the shadow dom specificities? Thank you!

@AllanJard
Copy link

Sounds like it might be cloning (mirroring?) elements, without copying across event listeners. Its not something I've looked into before I'm afraid. Is there a page I can load in my web browser where I can see the problem?

@mwouts
Copy link
Owner Author

mwouts commented Jun 3, 2025

Sounds like it might be cloning (mirroring?) elements, without copying across event listeners. Its not something I've looked into before I'm afraid. Is there a page I can load in my web browser where I can see the problem?

Thank you Allan! One of the selling points of Marimo is that it makes it easy to share examples, let me see if we can use this link ? NB: Loading everything takes a while, on my end the datatable appears after 20-30 seconds.

"""This is a Python script that modifies the dt_bundle.css to
add :host to each :root selector. This ensures that the styles
works in Marimo which uses Shadow DOM.
"""
Copy link
Owner Author

Choose a reason for hiding this comment

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

@AllanJard this is a script that adds host: selectors for every root: selector found in a given CSS file. At the current stage of this PR, I run this script on the CSS bundle produced by dt_for_itables, in a post process step.

That's fine with me but before merging I wanted to see what you think of the alternative, that would be to add explicit host: selectors in all the datatable CSS files. Is that something you would consider? If so I could use this script to prepare a PR (what would be the appropriate repo(s) to do the PR on, given that I use datatables and a few extensions?)

@mwouts mwouts marked this pull request as ready for review June 4, 2025 06:14
@AllanJard
Copy link

How interesting! The event handlers are working - it is the insertion of the elements into the document that isn't working. When you click a dropdown (Show 10 rows or Column visibility) the dropdown is inserted into the document to show the list of options.

The same is the case with the Copy button, the action is happening and the data is copied to clipboard, but the popover isn't inserted into the document (it uses the same method as linked above).

I presume that this is due to the shadow DOM. Possibly the insert is happening into the original, and it isn't reflected? I don't know - I haven't worked with shadow DOM much. I don't know if this is something that should be handled in Buttons, or if there should be an update listener somewhere else. Certainly DataTables and its extensions move and insert DOM elements at lot (e.g. just putting the buttons there is an insert) so I'm missing some context on how all of this hangs together.

@mwouts
Copy link
Owner Author

mwouts commented Jun 6, 2025

Thanks Allan! No problem at all, I'll merge this PR soon as it fixes the selected row style already, and for the dropdown insertion (e.g issue #387) we can see later on.

mwouts added 3 commits June 6, 2025 22:01
Tentative Python script that extends root to host
@mwouts mwouts merged commit db70324 into main Jun 6, 2025
15 checks passed
@mwouts mwouts deleted the css_in_marimo branch June 6, 2025 21:18
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.

4 participants