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

Fix indent and center #4708

Merged
merged 11 commits into from Jan 27, 2014
Merged

Conversation

damianavila
Copy link
Member

Some weeks ago they were merged two PR changing some UI things: #4154 and #4567.
These PR bring some changes in the centring of equations and indentation of non code cells.
This PR takes into account this changes and translate it to nbconvert.

Here is the screenshot of current html_full from nbconvert:

ui1

And this one is the new look proposed by this PR, which follow the last changes in the notebook appearance:

ui2

I have also regenerated the null.tplx because it was not updated (missing a lower() in the raw cell block).

Now, I think is ready for review.

@Carreau
Copy link
Member

Carreau commented Dec 17, 2013

Will read more carefully but at first glance null.tplx should be regenerated and test fails.

@damianavila
Copy link
Member Author

Yep, not ready for review yet... I did some things too much complicated... I will work on this in a couple of hours...

@damianavila
Copy link
Member Author

@Carreau simplified approach, test passed, and ready for review!

{% block e_in_prompt -%}
<div class="prompt input_prompt">
</div>
{%- endblock e_in_prompt %}
Copy link
Member

Choose a reason for hiding this comment

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

and what would be the meaning of e_ it would nice to be more explicit.

Copy link
Member Author

Choose a reason for hiding this comment

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

e is for empty... I try to make it short but I can change it if you want...

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I would prefer something more explicit. I'm searching for a better name, but can't find.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, think a little bit... if nobody come up with a better name (including me 😉) by tomorrow, I will change it to empty_in_prompt to make it more explicit...

@damianavila
Copy link
Member Author

Changed the name to of empty prompt to a more explicit one: empty_in_prompt 😉 instead of e_in_prompt.

@damianavila
Copy link
Member Author

Any other comments?

@damianavila
Copy link
Member Author

People, any more comments on this?

@ghost ghost assigned ellisonbg Jan 13, 2014
displayAlign: 'left', // Change this to 'center' to center equations.
// Center justify equations in code and markdown cells. Elsewhere
// we use CSS to left justify single line equations in code cells.
displayAlign: 'center',
"HTML-CSS": {
Copy link
Member

Choose a reason for hiding this comment

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

While you are at this, can you audit the other MathJax config things to make sure nbconvert is tracking what is in the live notebook. For example, we also now set linebreaks: { automatic: true } inside the HTML-CSS section. Might as well update all that while we are touching this code.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I will do it tomorrow morning... it's late here ;-)

@damianavila
Copy link
Member Author

OK, rebased (there was a lot of changes in html template lately) and @ellisonbg's comments addressed... ready for a re-review 😉

@takluyver
Copy link
Member

@ellisonbg , this is still assigned to you - do you think it needs any more review before it's merged?

@ellisonbg
Copy link
Member

Let me double check to see how it looks.

On Fri, Jan 24, 2014 at 11:31 AM, Thomas Kluyver
notifications@github.comwrote:

@ellisonbg https://github.com/ellisonbg , this is still assigned to you

  • do you think it needs any more review before it's merged?


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

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

@ghost ghost assigned jdfreder Jan 26, 2014
@ellisonbg
Copy link
Member

Looks good, merging.

ellisonbg added a commit that referenced this pull request Jan 27, 2014
@ellisonbg ellisonbg merged commit 1d0bcb1 into ipython:master Jan 27, 2014
@damianavila damianavila deleted the fix_indent_and_center branch January 27, 2014 19:56
@damianavila
Copy link
Member Author

Thanks!

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants