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.

Sign up for GitHub

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

Jump to bottom

fix for codemirror width #11805

Closed
wants to merge 2 commits into from
Closed

fix for codemirror width #11805

wants to merge 2 commits into from

Conversation

pree-T
Copy link
Contributor

@pree-T pree-T commented Jan 8, 2022

#11696 a possible fix for this. Kindly review

References

Code changes

User-facing changes

Backwards-incompatible changes

@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@welcome
Copy link

welcome bot commented Jan 8, 2022

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@github-actions github-actions bot added Design System CSS pkg:cells tag:CSS For general CSS related issues and pecadilloes labels Jan 8, 2022
@fcollonval
Copy link
Member

fcollonval commented Jan 9, 2022 •
edited

Open question: this changes the width of the input prompt to a constant width. Is this acceptable? Do we need to apply the same change in the output area (I would say yes)?

jupyterlab/packages/outputarea/style/base.css

Line 28 in 4a9c1d5

.jp-OutputPrompt {

fcollonval
fcollonval requested changes Jan 9, 2022
View reviewed changes
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @pree-T I made some suggestions. And they are one question regarding the need to apply similar changes to the output area that is likely to be confirmed by others.

packages/cells/style/inputarea.css Outdated
@@ -59,6 +62,7 @@
.jp-InputArea-editor {
display: table-row;
margin-left: var(--jp-notebook-padding);

Copy link
Member

Choose a reason for hiding this comment

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

Unneeded empty line

Suggested change

packages/cells/style/inputarea.css Outdated
@@ -40,6 +42,7 @@
font-size: var(--jp-code-font-size);
border: var(--jp-border-width) solid transparent;
opacity: var(--jp-cell-prompt-opacity);
width: var(--jp-cell-prompt-width);
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove the min-width rule?

packages/cells/style/inputarea.css Outdated
}

.jp-InputArea-editor {
display: table-cell;
width: 100%;

Copy link
Member

Choose a reason for hiding this comment

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

Unneeded empty line

Suggested change

tsconfigdoc.json Outdated
@@ -20,274 +20,274 @@
},
"references": [
{
"path": "./packages/application"
"path": "./packages\\application"
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind reverting the change in this file please?

@fcollonval
Copy link
Member

I looked again at the problematic PR https://github.com/jupyterlab/jupyterlab/pull/11508/files

Actually the error comes from the style flex: 0 0 <width> that have been replaced by a min-width constrain when they should have been replaced by width constraint.

So could you please replace min-width by width in both:

jupyterlab/packages/outputarea/style/base.css

Line 31 in 4a9c1d5

min-width: var(--jp-cell-prompt-width);

jupyterlab/packages/cells/style/widget.css

Line 96 in 4a9c1d5

min-width: calc(

And set that table as table-layout: fixed; as you did for the input area.

jupyterlab/packages/outputarea/style/base.css

Line 22 in 4a9c1d5

.jp-OutputArea-child {

@pree-T
Copy link
Contributor Author

pree-T commented Jan 9, 2022

Sure Sir, I'll make all the necessary changes. Thanks for the guidance .

@pree-T pree-T closed this Jan 10, 2022
@pree-T pree-T mentioned this pull request Jan 10, 2022
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@fcollonval fcollonval fcollonval requested changes

Assignees

@pree-T pree-T

Labels
Design System CSS pkg:cells pkg:outputarea tag:CSS For general CSS related issues and pecadilloes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pree-T @fcollonval