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

Migrate from Bootstrap 2 to Bootstrap 3 #5617

Merged
merged 54 commits into from
Jun 10, 2014
Merged

Conversation

jdfreder
Copy link
Member

Sibling PR in components: jupyter/ipython-components#24

@jdfreder
Copy link
Member Author

Whoops, just noticed because of GFM I just referenced a million different PR/Issues here. Sorry all- will try to fix.

@minrk
Copy link
Member

minrk commented Apr 15, 2014

This PR should wait for #4536.

@jdfreder
Copy link
Member Author

For convenience, here are some screenshots from before the upgrade:
before2


before5


before4


before3


before1

@jdfreder
Copy link
Member Author

Here are the after shots:
(These screenshots are stale.)
after1


after2


after3


after4


after5

@jdfreder
Copy link
Member Author

Linking to #5556 since it looks like that uses BS3 too.

@damianavila damianavila mentioned this pull request May 1, 2014
@minrk minrk added this to the 3.0 milestone May 7, 2014
@jdfreder
Copy link
Member Author

This PR should wait for #4536.

Now that this is in, I'm going to start to rebase this PR. After rebasing it, I'll dump a list of the LESS & CSS classes so we can discuss the CSS/LESS refactor.

@jdfreder
Copy link
Member Author

I went ahead and rebased this, ouch. Here are two dicts which contain the LESS and CSS variable names found within the IPython code. I'm going to go through these by hand and make a recommendation for the LESS/CSS refactoring in this PR.
...

@jdfreder
Copy link
Member Author

It was decided at the weekly Google Hangout dev meeting to postpone the CSS/LESS refactor for another PR. I've pushed the sibling PR ( jupyter/ipython-components#24 ) so people can start to play around with this if they want.

@jdfreder
Copy link
Member Author

I wasn't sure why the shadow on the toolbar was upwards, so I've been experimenting with having the shadow below the toolbar so it looks like the toolbar is floating above the document:

screen shot 2014-05-15 at 12 45 11 pm

Regarding the same screenshot, I'm not really sure what to do about the menu either. Should the edges be aligned to the rest of the document, or instead the text inside it (i.e. "File")?

@jdfreder
Copy link
Member Author

For anyone wondering, this is what it looks like when "File" is left aligned to the rest of the content:

screen shot 2014-05-15 at 12 52 47 pm

@jdfreder
Copy link
Member Author

Also, I don't know if we want to spend much time on it in this PR, but I do want to point out that there are at least 4 different 5 different 6 different 7 different shades of grey here (and that's not counting the different greys in the cell coloring!). I tried to reduce that a bit by making the toolbar border the same as the menubar border:

screen shot 2014-05-15 at 12 52 47 pm

@jdfreder
Copy link
Member Author

One last note, upon request of @ellisonbg I've been playing with increasing the vertical padding between the top of the document and the toolbar. The screenshot shows the page scrolled all the way to the top, no blank cells between the header and the top of the document.

@jdfreder jdfreder changed the title [WIP] Migrate from Bootstrap 2 to Bootstrap 3 Migrate from Bootstrap 2 to Bootstrap 3 May 15, 2014
@ellisonbg
Copy link
Member

Umm, I don't recall asking for additional vertical padding between the top
of the document and the toolbar. Hmm, when did I say this? I definitely
don't think we want to do this.

On Thu, May 15, 2014 at 1:05 PM, Jonathan Frederic <notifications@github.com

wrote:

One last note, upon request of @ellisonbg https://github.com/ellisonbgI've been playing with increasing the vertical padding between the top of
the document and the toolbar. The screenshot shows the page scrolled all
the way to the top, no blank cells between the header and the top of the
document.


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

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

@ellisonbg
Copy link
Member

I will stop by tomorrow so we can run through some of the visual
aspects...easier to do sitting in front of a single screen.

On Thu, May 15, 2014 at 4:46 PM, Brian Granger ellisonbg@gmail.com wrote:

Umm, I don't recall asking for additional vertical padding between the top
of the document and the toolbar. Hmm, when did I say this? I definitely
don't think we want to do this.

On Thu, May 15, 2014 at 1:05 PM, Jonathan Frederic <
notifications@github.com> wrote:

One last note, upon request of @ellisonbg https://github.com/ellisonbgI've been playing with increasing the vertical padding between the top of
the document and the toolbar. The screenshot shows the page scrolled all
the way to the top, no blank cells between the header and the top of the
document.


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

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

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

@jdfreder
Copy link
Member Author

Umm, I don't recall asking for additional vertical padding between the top
of the document and the toolbar. Hmm, when did I say this? I definitely
don't think we want to do this.

I must have misunderstood you, I thought when we were talking in the context of the space/shift-space slide scrolling plugin you had said we should add some more space there... I'll go ahead and remove it, it looks awkwardly large anyways (2em vs 1em).

@jdfreder
Copy link
Member Author

Updated screenshot with old doc padding and HTML5 selects in the tb:

screen shot 2014-05-16 at 10 42 07 am

@jdfreder
Copy link
Member Author

@ellisonbg here is some of the changes we talked about in person (the grey line at the top is the top of the browser render area [not part of the actual notebook]):

screen shot 2014-05-16 at 1 56 44 pm


screen shot 2014-05-16 at 1 56 00 pm

@takluyver
Copy link
Member

@ivanov suggested yesterday that we're aiming for 50 shades

@ellisonbg
Copy link
Member

That would open up a huge new audience for the project...not sure we want
that

On Fri, May 16, 2014 at 2:29 PM, Thomas Kluyver notifications@github.comwrote:

@ivanov https://github.com/ivanov suggested yesterday that we're aiming
for 50 shades


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

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

@damianavila
Copy link
Member

That would open up a huge new audience for the project...not sure we want
that

Jajaja

@jdfreder
Copy link
Member Author

This is ready for some review. Travis won't pass since the sibling components PR isn't merged.

@ellisonbg
Copy link
Member

From in person review:

Dashboard

  • div#ipython_notebok has different font-size and line-height?
  • ul#tabs a {padding-top: 6px;}
  • in tree/css/override.css set ipython-main-app top margin to 28px to match
    tab height.

Modals

  • Try to improve look of modals by 1) removing any hack styling we already have
    and 2) by just setting bootstrap 3 variables in base/less/variables.less.
  • No modal animation :(

Colors

  • use @dark_dashboard_color for @breadcrumb_color
  • Define @black to be real black and use that as @text_color everywhere

Notebook

  • Keyboard shortcut alert color should be yellow on the first paragraph.
  • div.prompt {min-width: 15ex;}
  • .celltoolbar {height: 29px;}
  • .celltoolbar>div {padding-top: 3px;}
  • Edit Cell Metadata model: text should be non-bold, refresh CodeMirror.
  • Try global .label {font-weight: normal;} and remove per element styling.
  • Fix label of raw cell and slide show ctb
  • Fix height of dropdowns in celltoolbars to match the button
  • Watch right margins of dropdowns - match to button.
  • In rename modal, the text area is not bootstrap
  • Border at the top of the notebook (below the toolbar) should match the border
    that is at the top of the menubar.
  • .rendered_html ul {padding-left: 0px;} same for ol
  • @blockquote-font-size to inherit

@jdfreder
Copy link
Member Author

jdfreder commented Jun 9, 2014

Comments addressed... I'm going to go ahead and begin the painful process of rebasing this now.

@jdfreder
Copy link
Member Author

jdfreder commented Jun 9, 2014

Rebase successful 🎆

@ellisonbg
Copy link
Member

  • Dropdown menu in Slide show celltoorbar should be 22ox

@ellisonbg
Copy link
Member

I like the current look. Two things that remain:

  • Get the Keyboard Shortcuts phase of the Tour working.
  • Looks like most elements in the notebook content have z-index: auto and appear on top of the shadow. Maybe set the zindex of div#notebook so everything appears below the shadow?

element: "#checkpoint_status",
title: "Checkpoint Status",
placement: 'bottom',
content: "Information about the last time this notebook was saved."
}, {
},*/ {
Copy link
Member

Choose a reason for hiding this comment

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

why the block quote?

Copy link
Member

Choose a reason for hiding this comment

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

This block should just be removed.

On Tue, Jun 10, 2014 at 10:28 AM, Min RK notifications@github.com wrote:

In IPython/html/static/notebook/js/tour.js:

 element: "#checkpoint_status",
 title: "Checkpoint Status",
 placement: 'bottom',
 content: "Information about the last time this notebook was saved."
  • }, {
  • },*/ {

why the block quote?


Reply to this email directly or view it on GitHub
https://github.com/ipython/ipython/pull/5617/files#r13607108.

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

ellisonbg added a commit that referenced this pull request Jun 10, 2014
Migrate from Bootstrap 2 to Bootstrap 3
@ellisonbg ellisonbg merged commit f1ad8aa into ipython:master Jun 10, 2014
@ellisonbg
Copy link
Member

Woohoo!

@jdfreder
Copy link
Member Author

Discussed during the office hours, we will continue to investigate this:

  • Looks like most elements in the notebook content have z-index: auto and appear on top of the shadow. Maybe set the zindex of div#notebook so everything appears below the shadow?

@jdfreder jdfreder deleted the bootstrap3 branch June 10, 2014 17:53
@takluyver
Copy link
Member

This broke installation, because the path to bootstrap.min.js in package_data needed to change. I've fixed it (hopefully) in #5971.

@jdfreder jdfreder mentioned this pull request Jun 10, 2014
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Migrate from Bootstrap 2 to Bootstrap 3
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.

5 participants