jupytercad / JupyterCAD

A JupyterLab extension for collaborative 3D geometry modeling
https://jupytercad.github.io/JupyterCAD
BSD 3-Clause "New" or "Revised" License
141 stars 18 forks source link

Toggle buttons for clipplane and wireframe do not toggle in JupyterLite #488

Closed SylvainCorlay closed 3 weeks ago

SylvainCorlay commented 1 month ago

We could decide to make the clipping toolbar icon should appear as toggled when clipping is enabled.

martinRenou commented 1 month ago

This was implemented in https://github.com/jupytercad/JupyterCAD/pull/441 and it looks like it's not working anymore?

martinRenou commented 1 month ago

I wonder if the switch to reactive toolbar may broke it https://github.com/jupytercad/JupyterCAD/pull/455

SylvainCorlay commented 1 month ago

Maybe @brichet would know if the reactive toolbar does not support toggled buttons.

arjxn-py commented 1 month ago

Hi there, I tried reproducing this on my end and It works fine for me (I'm at the latest commit)

Maybe can you try reproducing?

https://github.com/user-attachments/assets/bb4ed69e-fb0f-4b4c-b00c-7eb8939041fb

EDIT: Yes, I just realised and checked that it's not working on jupyterlite deployment

brichet commented 1 month ago

Maybe @brichet would know if the reactive toolbar does not support toggled buttons.

AFAIK the reactive toolbar handles the buttons in the same way as the regular toolbar, storing only those that overflow into a popup toolbar.

SylvainCorlay commented 1 month ago

EDIT: Yes, I just realised and checked that it's not working on jupyterlite deployment

Indeed. Although I can see the class change on the button.

martinRenou commented 1 month ago

So this may be JupyterLite not bringing the Lumino CSS for lm-mod-toggled!

SylvainCorlay commented 1 month ago

So this may be JupyterLite not bringing the Lumino CSS for lm-mod-toggled!

Ping @jtpio

jtpio commented 1 month ago

hmm strange that JupyterLite would not include this rule, since it should include the Lumino CSS by default.

Are you able to check the rule is available when using regular JupyterLab, and not with JupyterLite? (for example via the elements tab of the dev tools)

arjxn-py commented 1 month ago

Are you able to check the rule is available when using regular JupyterLab, and not with JupyterLite? (for example via the elements tab of the dev tools)

Hi @jtpio, yes this is available on regular JupyterLab, this clip is of JupyterLab running locally.

jtpio commented 1 month ago

I mean can you point at the specific CSS rule? (so it's easier to check why it would be missing in lite)

arjxn-py commented 1 month ago

It should be lm-mod-toggled

https://github.com/user-attachments/assets/9843fdac-193d-474c-9512-f628a9992025

jtpio commented 1 month ago

For the class yes. But what is the expected CSS rule?

arjxn-py commented 1 month ago

For the class yes. But what is the expected CSS rule?

This is the only rule we could find in lumino -

https://github.com/jupyterlab/lumino/blob/10ee22d2a93ae618f36c5ba380337bc440106399/packages/default-theme/style/menu.css#L75-L77

jtpio commented 1 month ago

So to echo the question above:

Are you able to check the rule is available when using regular JupyterLab, and not with JupyterLite? (for example via the elements tab of the dev tools)

Is this rule on the page for JupyterLab, and not for JupyterLite?

arjxn-py commented 1 month ago

I cannot see this exact rule but there are significant differences between how lab is rendering the button and how lite is. One of the major one is that in jupyterlab button is jp-button but in jupyterlite it's only button and i also cannot notice styles being modified in lite

JupyterLab

https://github.com/user-attachments/assets/ec2b2c08-0ddf-4ba9-b892-34a65172e35c

JupyterLite

https://github.com/user-attachments/assets/f64e5b47-4eb7-4db1-b1fe-578ea6959920

jtpio commented 1 month ago

I cannot see this exact rule but there are significant differences between how lab is rendering the button and how lite is.

Even by searching for lm-mod-toggled on the "Elements" tab? Probably there should then be some references to it in the <style> elements.

arjxn-py commented 1 month ago

I cannot see this exact rule but there are significant differences between how lab is rendering the button and how lite is.

Even by searching for lm-mod-toggled on the "Elements" tab? Probably there should then be some references to it in the <style> elements.

Can't find one in local jupyterlab:

https://github.com/user-attachments/assets/6f8f22ac-dba2-4493-96fc-95a5b0df108a

But in jupyterlite, is this it? - Image

jtpio commented 1 month ago

One of the major one is that in jupyterlab button is jp-button but in jupyterlite it's only button

Indeed. It looks related to the changes in JupyterLab w.r.t the toolbar buttons. Maybe there is a @jupyterlab package in JupyterLite that was not updated to the latest version?

martinRenou commented 1 month ago

Even though it is visible in JupyterLab, I personally find the shadow styling a bit ugly:

Image

I'd like to suggest to apply a background-color: var(--jp-layout-color3); to the toggled button instead of a shadow:

Image

That's just a personal preference.

SylvainCorlay commented 1 month ago

Will the "bug" be resolved in a future version of JupyterLite ?

The CSS change could be submitted upstream in Lab.

arjxn-py commented 1 month ago

Will the "bug" be resolved in a future version of JupyterLite ?

From a conversation with @martinRenou on video, Yes

The CSS change could be submitted upstream in Lab.

Sounds good, happy to do that if I'm good to do that, please let know if someone else shall do it.

jtpio commented 1 month ago

Following up on this: is there anything to be done in JupyterLab? In JupyterLite? Is there an issue in these repos?

martinRenou commented 1 month ago

Following up on this: is there anything to be done in JupyterLab? In JupyterLite? Is there an issue in these repos?

From that thread, two issues are have been raised:

martinRenou commented 1 month ago

It looks like the rule that is being used for that toggle button is:

Image

It is nowhere to be found though!!! This "constructed" thingy leads to nothing in my dev tools, and I can't find any reference to this variable in JupyterLab or Lumino. I can only find that this CSS variable comes from microsoft/fast, not the rule though.

martinRenou commented 3 weeks ago

I'm removing this issue from the 3.0.0 milestone for JupyterCAD.

How about we introduce a new CSS rule in JupyterLab for this, that implements what I suggested in https://github.com/jupytercad/JupyterCAD/issues/488#issuecomment-2431174872, then JupyterLite will get this for free upon update at some point.

martinRenou commented 3 weeks ago

Out of nowhere, it seems to be working now...

Image