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

remove class jupyter-widgets from jp-OutputArea-output node #2500

Merged
merged 1 commit into from
Feb 11, 2020

Conversation

qzchenwl
Copy link
Contributor

Fix issue #2499

@jasongrout
Copy link
Member

jasongrout commented Jul 29, 2019

Thanks!

The jupyter-widgets class is meant to be the very base class for jupyter widgets that sets some CSS defaults (such as font color, etc.). However, I notice that it is set a lot in the code, at multiple levels. I think we need to be more deliberate about how we are using it (e.g., is it convention that it is applied at the top-level widget? At the root node of each widget? Above the entire widget hierarchy?)

Also, perhaps we could just change the overflow setting. It was set to visible in 2016 (#919).

Whatever change we decide on, I think we need to carefully examine lots of examples using widgets to understand the ramifications of this change. We've seen this before with seemingly small changes that had large ramifications, especially in CSS.

@SylvainCorlay SylvainCorlay self-assigned this Jan 8, 2020
@SylvainCorlay
Copy link
Member

I think that the jupyter-widgets CSS class is only meant for the top-level DOM elements.

I would be in favor of this, but I need to double check that everything is fine in the PR.

@jasongrout
Copy link
Member

I did some initial investigations. Visually, removing this class removes a layer of padding around the entire set of widgets.

I'm a little curious about the css for this class of overflow: visible, which is the main point here. In general, it seems odd to let the content flow out of the widget's containing box, so I'm not sure why it is set at all. Tracing back through the history, it seems this overflow: visible was added in #796, and it's not clear what problem it was solving.

@jasongrout
Copy link
Member

I'm a little curious about the css for this class of overflow: visible, which is the main point here. In general, it seems odd to let the content flow out of the widget's containing box, so I'm not sure why it is set at all. Tracing back through the history, it seems this overflow: visible was added in #796, and it's not clear what problem it was solving.

But this is a different issue than this one, i.e., .jupyter-widgets being applied at the top level.

Let's merge this and address any other problems that come up. Thanks again @qzchenwl!

@jasongrout jasongrout merged commit 38af082 into jupyter-widgets:master Feb 11, 2020
@lock lock bot added the resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label May 20, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backwards-incompatible resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants