Skip to content
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

CSS issue when the Notebook windowing mode is set to full #7231

Closed
jtpio opened this issue Feb 1, 2024 · 24 comments · Fixed by #7321 or #7337
Closed

CSS issue when the Notebook windowing mode is set to full #7231

jtpio opened this issue Feb 1, 2024 · 24 comments · Fixed by #7321 or #7337
Labels
Milestone

Comments

@jtpio
Copy link
Member

jtpio commented Feb 1, 2024

Testing on the latest main, which includes the recent changes that landed in JupyterLab w.r.t windowing mode, it looks like the CSS is off:

image

This is likely related to an underlying DOM structure change, which affects the CSS customizations in Notebook.

@jupyterlab-probot jupyterlab-probot bot added the status:Needs Triage Applied to issues that need triage label Feb 1, 2024
@jtpio jtpio added this to the 7.1.x milestone Feb 1, 2024
@jtpio jtpio added the bug label Feb 1, 2024
@jtpio
Copy link
Member Author

jtpio commented Feb 1, 2024

Adding to 7.1.x since full is not the default windowing mode.

@jtpio
Copy link
Member Author

jtpio commented Feb 27, 2024

Marking as a "good first issue" if someone would like to have a look:

The windowing mode can be set to full via the settings editor:

image

This will also likely require tweaking some CSS depending on the DOM structure: https://github.com/jupyter/notebook/blob/main/packages/notebook-extension/style/base.css

@kgaurav8026
Copy link

Hi @jtpio,
I’m interested in contributing to JupyterLab, and I’d like some additional context regarding the issue related to cell alignment. From what I understand, the issue pertains to the misalignment of the cell box when windowing. Specifically, should the cell box be centered on the screen?

@jtpio
Copy link
Member Author

jtpio commented Mar 8, 2024

Yes it should be centered like when using the default mode:

image

@v-lozko
Copy link
Contributor

v-lozko commented Mar 11, 2024

Hello, would it be possible to have this assigned to me?

Thanks

@kgaurav8026
Copy link

Hello, would it be possible to have this assigned to me?

Thanks

I'm also working to resolve the issue. If you find a solution, you can submit a PR I guess. Assigning it won't be beneficial.

@v-lozko
Copy link
Contributor

v-lozko commented Mar 11, 2024

It will be so efforts arent duplicated, so work on it. if you can't/dont have time let me know and i can take a stab at it

@kgaurav8026
Copy link

It will be so efforts arent duplicated, so work on it. if you can't/dont have time let me know and i can take a stab at it

Ok sure. I will let you know if I'm not able to complete it. I'm currently halfway there

@kgaurav8026
Copy link

Hi @v-lozko , i am finding it difficult to fix the bug. So you can start working on it if you want.
Thanks

@kgaurav8026
Copy link

@v-lozko Also, if you are able to merge Kindly tell me the approach as i am keen on understanding how to tackle such issues in the future.

@v-lozko
Copy link
Contributor

v-lozko commented Mar 18, 2024

Hello @jtpio i can't seem to replicate the issue. This is the first issue i'm dealing with in Jupyter, but it definitely seems like i set up the environment correctly, and having changed my settings to full i do not have the CSS issue that you shared. it looks fine.

@jtpio
Copy link
Member Author

jtpio commented Mar 19, 2024

I just tried from the latest commit (851a4be) and it still seems to be an issue:

image

@v-lozko
Copy link
Contributor

v-lozko commented Mar 19, 2024

huh, i have windowing set to full
image
and here is what i'm seeing
image

@jtpio
Copy link
Member Author

jtpio commented Mar 19, 2024

Did you refresh the notebook page after changing the setting?

@v-lozko
Copy link
Contributor

v-lozko commented Mar 19, 2024

yes, and i've tried reinstalling the build multiple times. i see you're using gitpod based on your url, could that be a possible source of the issue?

@jtpio
Copy link
Member Author

jtpio commented Mar 19, 2024

Here is a screencast to reproduce the issue, testing on Binder with Notebook 7.1.2 from this gist: https://gist.github.com/jtpio/d368ab89cee5123ecee60683115e15f3

notebook-full-windowing-mode.webm

you're using gitpod based on your url, could that be a possible source of the issue?

This was tested in a fresh new environment on Gitpod, so not sure this is the reason. And it is also reproducible on Binder (which also creates a clean new environment).

@v-lozko
Copy link
Contributor

v-lozko commented Mar 19, 2024

So i went out, got a new macbook as i was previously using a windows, set everything up and this time tried it both with chrome and safari as well as binder. i get the issue with safari and binder, but i am not able to repeat it on chrome. so at least this gives me a place to start

@jtpio
Copy link
Member Author

jtpio commented Mar 20, 2024

Thanks @v-lozko for looking into this.

I also checked on Chromium (on Ubuntu) and it also seems to be the case:

image

@v-lozko
Copy link
Contributor

v-lozko commented Mar 20, 2024

Thanks for your patience as well, I have been a bit busy these past couple days but had a chance to look into a little. I'm not certain its the DOM, as it seems the right margin for the windowed panel is being overridden by a style attribute coming from somewhere (which surprisingly isn't happening with chrome). I'll get it sorted in next day or so, someone much better versed in CSS will help me sort it out.

@Dev-Akash
Copy link

Hi @v-lozko , I hope I can help on the CSS part in this issue, if there is a room for contribution please let me know.
Thank you!

@v-lozko
Copy link
Contributor

v-lozko commented Mar 21, 2024

Hey @Dev-Akash, I have put some time in already trying to resolve this and would like to see it through in next day or so. If I can't figure it out I can definitely let you know! I appreciate the offer

@Dev-Akash
Copy link

Sure! @v-lozko.
Thank you!

v-lozko added a commit to v-lozko/notebook that referenced this issue Mar 22, 2024
Updating to limit max width of inner windowed panel
@v-lozko
Copy link
Contributor

v-lozko commented Mar 22, 2024

@jtpio i submitted a PR here for this issue however i forgot to appropriately label it so its not tied to this issue. Is there a way to edit the labels post submittal of the PR?

@tentomush1
Copy link

Is the issue solved yet? what is the progress?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants