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

Tighten up the vertical spacing on cells and make the padding of cells more consistent #4576

Merged
merged 5 commits into from Dec 6, 2013

Conversation

ellisonbg
Copy link
Member

Previously, the padding and margins on cells were all over the place. There was also extra vertical padding that took up too much vertical space, even for simple cells. Here is a summary of the changes.

  • Introduce a @code_padding variable that is 0.4em. This is the padding we use in the CodeMirror editor and has to be used in various places to get a consistent design.
  • Removed 2px margin between cells.
  • Moved some output related less code from cell.less to outputarea.less.
  • Removed the 5px top margin from the output wrapper. This was causing the output area to occupy vertical space even when empty.
  • All output_subarea instances now have the @code_padding on the top and sides. This means that there is a consistent 0.4em spacing between different outputs in the same cell. This is less that the 10px spacing that was there before.

The overall visual look is slightly tighter vertically.

Here is a screenshot before:

before

And after:

after

@Carreau
Copy link
Member

Carreau commented Nov 22, 2013

Can you make code_pading CamelCase to be consistent with bootstrap ? I've started to correct some other variables in my last css cleaning, would be good to continue in this direction.

Envoyé de mon iPhone

Le 22 nov. 2013 à 01:45, "Brian E. Granger" notifications@github.com a écrit :

Previously, the padding and margins on cells were all over the place. There was also extra vertical padding that took up too much vertical space, even for simple cells. Here is a summary of the changes.

Introduce a @code_padding variable that is 0.4em. This is the padding we use in the CodeMirror editor and has to be used in various places to get a consistent design.
Removed 2px margin between cells.
Moved some output related less code from cell.less to outputarea.less.
Removed the 5px top margin from the output wrapper. This was causing the output area to occupy vertical space even when empty.
All output_subarea instances now have the @code_padding on the top and sides. This means that there is a consistent 0.4em spacing between different outputs in the same cell. This is less that the 10px spacing that was there before.
The overall visual look is slightly tighter vertically.

Here is a screenshot before:

And after:

You can merge this Pull Request by running

git pull https://github.com/ellisonbg/ipython cell-spacing
Or view, comment on, or merge it at:

#4576

Commit Summary

Making the cell margin 0 all around.
Adjusting padding of output subareas and adding @code_padding.
File Changes

M IPython/html/static/notebook/less/cell.less (6)
M IPython/html/static/notebook/less/codecell.less (43)
M IPython/html/static/notebook/less/codemirror.less (4)
M IPython/html/static/notebook/less/outputarea.less (50)
M IPython/html/static/notebook/less/pager.less (2)
M IPython/html/static/notebook/less/variables.less (1)
M IPython/html/static/style/ipython.min.css (17)
M IPython/html/static/style/style.min.css (17)
Patch Links:

https://github.com/ipython/ipython/pull/4576.patch
https://github.com/ipython/ipython/pull/4576.diff

@Carreau
Copy link
Member

Carreau commented Nov 22, 2013

Actually I might have just replaced some of our variable with bootstrap ones,(like BorderRadius I guess) so never mind.

Envoyé de mon iPhone

Le 22 nov. 2013 à 01:45, "Brian E. Granger" notifications@github.com a écrit :

Previously, the padding and margins on cells were all over the place. There was also extra vertical padding that took up too much vertical space, even for simple cells. Here is a summary of the changes.

Introduce a @code_padding variable that is 0.4em. This is the padding we use in the CodeMirror editor and has to be used in various places to get a consistent design.
Removed 2px margin between cells.
Moved some output related less code from cell.less to outputarea.less.
Removed the 5px top margin from the output wrapper. This was causing the output area to occupy vertical space even when empty.
All output_subarea instances now have the @code_padding on the top and sides. This means that there is a consistent 0.4em spacing between different outputs in the same cell. This is less that the 10px spacing that was there before.
The overall visual look is slightly tighter vertically.

Here is a screenshot before:

And after:

You can merge this Pull Request by running

git pull https://github.com/ellisonbg/ipython cell-spacing
Or view, comment on, or merge it at:

#4576

Commit Summary

Making the cell margin 0 all around.
Adjusting padding of output subareas and adding @code_padding.
File Changes

M IPython/html/static/notebook/less/cell.less (6)
M IPython/html/static/notebook/less/codecell.less (43)
M IPython/html/static/notebook/less/codemirror.less (4)
M IPython/html/static/notebook/less/outputarea.less (50)
M IPython/html/static/notebook/less/pager.less (2)
M IPython/html/static/notebook/less/variables.less (1)
M IPython/html/static/style/ipython.min.css (17)
M IPython/html/static/style/style.min.css (17)
Patch Links:

https://github.com/ipython/ipython/pull/4576.patch
https://github.com/ipython/ipython/pull/4576.diff

@damianavila
Copy link
Member

Looks OK for me...
Just a comment... sometimes I saw two spaces indentation and sometimes 4 spaces indentation... I think we have to unify the convention and use just one of them for css and less code ;-)

@Carreau
Copy link
Member

Carreau commented Nov 22, 2013

Just a comment... sometimes I saw two spaces indentation and sometimes 4 spaces indentation

Good point.

@ellisonbg
Copy link
Member Author

@Carreau I think we should use not_camel_case for the less variables that we manage (non-Bootstrap ones). Otherwise it is really difficult to see which belong to Bootstrap and which belong to us.

@ellisonbg
Copy link
Member Author

OK I have fixed the cell spacing. @fperez or @minrk thoughts on this design change?

@damianavila
Copy link
Member

@ellisonbg variable.less looks great now, but I am also asking you to unify the spaces in all the less files you are modifying 😉

@ellisonbg
Copy link
Member Author

OK I think I got them all...

@damianavila
Copy link
Member

Almost there, you have left mixed indentation in codemirror.less and outputarea.less. Sorry to be an annoying guy (I can do a PR against yours if you want, but it is probably easier to you to modify it because you have the files open right now 😬)

@ellisonbg
Copy link
Member Author

Do you mean that the number of spaces used is different (sometimes 2,
sometimes 4)?

On Fri, Nov 22, 2013 at 2:35 PM, Damián Avila notifications@github.comwrote:

Almost there, you have left mixed indentation in codemirror.less and
outputarea.less. Sorry to be an annoying guy (I can do a PR against yours
if you want, but it is probably easier to you to modify it because you have
the files open right now [image: 😬])


Reply to this email directly or view it on GitHubhttps://github.com//pull/4576#issuecomment-29114761
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@damianavila
Copy link
Member

Yes, I told you I am an annoying guy, hehe... It is not a big deal, but improves the readability of the code (at least for me :bowtie:)

@ellisonbg
Copy link
Member Author

I will fix it - no problem...

On Fri, Nov 22, 2013 at 2:43 PM, Damián Avila notifications@github.comwrote:

Yes, I told you I am an annoying guy, hehe... It is not a big deal, but
improves the readability of the code (at least for me [image: :bowtie:])


Reply to this email directly or view it on GitHubhttps://github.com//pull/4576#issuecomment-29115220
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@Carreau
Copy link
Member

Carreau commented Dec 5, 2013

@ellisonbg did you had time to uniformise tabs ?

@fperez
Copy link
Member

fperez commented Dec 5, 2013

Love it!! Haven't looked at code, but visual impact is excellent. If code review passes, +1 on merge.

@Carreau
Copy link
Member

Carreau commented Dec 5, 2013

Code review is ok. except for tabs consistency !

@ellisonbg
Copy link
Member Author

OK I have fixed the indentation and spacing of all the less files touched. This should be ready for merging.

@damianavila
Copy link
Member

Yep! +1 for merge... thanks for taking care of the indentation and spacing consistency! 👍

Carreau added a commit that referenced this pull request Dec 6, 2013
Tighten up the vertical spacing on cells and make the padding of cells more consistent
@Carreau Carreau merged commit f7b599e into ipython:master Dec 6, 2013
@Carreau
Copy link
Member

Carreau commented Dec 6, 2013

Merged; Thanks !

@ellisonbg ellisonbg deleted the cell-spacing branch January 28, 2014 17:43
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Tighten up the vertical spacing on cells and make the padding of cells more consistent
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.

None yet

4 participants