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

FF Fix: alignment and scale of text widget #5037

Merged
merged 5 commits into from Feb 21, 2014

Conversation

jdfreder
Copy link
Member

@jdfreder jdfreder commented Feb 5, 2014

Fixes #5013

@ellisonbg
Copy link
Member

Hmm, the test of Travis Python 3 is failing in the terminal section - should be completely unrelated. Can you trigger it to run again?

@ellisonbg
Copy link
Member

It still looks a bit off at least for the sliders. FF on the R:

screen shot 2014-02-05 at 6 53 46 pm

The space between the handles is different, but the height of the slider track is also different. What about the following. Let's create a "super widget" that is a container with all the different types of widgets inside of it in different arrangements, with/wo descriptions, etc. We can then use that to do a more complete visual audit of the styles and compare C + FF.

@jdfreder
Copy link
Member Author

jdfreder commented Feb 6, 2014

Can you trigger it to run again?

Just tried, looks like you already did?

@takluyver
Copy link
Member

I've prodded it to restart. @ellisonbg , any of us should be able to restart Travis jobs on the ipython repo. However, a failure in terminal is manifestly not relevant - let's not get into the habit of always waiting for a green indicator from Travis.

@jdfreder
Copy link
Member Author

Presenting lé super widget

screenshot from 2014-02-10 09 01 53

@Carreau
Copy link
Member

Carreau commented Feb 10, 2014

If you are showcasing.... completions with default values.

capture decran 2014-02-10 a 18 11 33

@jdfreder
Copy link
Member Author

If you are showcasing.... completions with default values.

Very cool!

@jdfreder
Copy link
Member Author

This PR is now rebased on top of #5075 - so don't think about merging this until that gets merged!

@jdfreder
Copy link
Member Author

After tweaking some more, and merging the latest from the new fbox model PR...
The left is Chrome and the right is FF:

c_ff_superwidget

The text area is a different height for the same number of lines (something different about how FF calculates it from the font size). I don't think that I can fix that. Correcting the text area height in my image editor lets us compare the rest of the elements...

c_ff_superwidget_lineheight

@jdfreder
Copy link
Member Author

Note: There are much much more side by side comparisons generated by the super widget. I will continue to look at them more.

@jdfreder
Copy link
Member Author

This should be ready to merge when the tests are done, do you want to look at this again @ellisonbg - I just looked at FF25 and Chrome31 and the widgets all align correctly. Here is the "superwidget" -> https://gist.github.com/jdfreder/9103330

@ellisonbg
Copy link
Member

Tests are failing on this one. After lunch I will start to work on code review...

@jdfreder
Copy link
Member Author

Whoops, looks like there was a small typo

@jdfreder
Copy link
Member Author

It passes now

@ellisonbg
Copy link
Member

Great! I have tested this on FF and Chrome and all looks great. Merging.

ellisonbg added a commit that referenced this pull request Feb 21, 2014
FF Fix: alignment and scale of text widget
@ellisonbg ellisonbg merged commit 144f5e9 into ipython:master Feb 21, 2014
@jdfreder
Copy link
Member Author

Thanks!

@jdfreder jdfreder deleted the ff-widget-align branch March 10, 2014 18:42
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
FF Fix: alignment and scale of text widget
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.

Widget alignment differs between FF and Chrome
4 participants