Skip to content

[MRG] Align binder badge with other download buttons#329

Merged
lesteve merged 2 commits intosphinx-gallery:masterfrom
lesteve:align-binder-badge
Jan 8, 2018
Merged

[MRG] Align binder badge with other download buttons#329
lesteve merged 2 commits intosphinx-gallery:masterfrom
lesteve:align-binder-badge

Conversation

@lesteve
Copy link
Copy Markdown
Member

@lesteve lesteve commented Dec 19, 2017

The binder badge stands a bit on its own and I feel it should be on the same footing as the other download button (download .py + download .ipynb).

About the CSS part, I copied and pasted sphx-glr-download. The thing is that there the rule div.sphx-glr-download a was coloring the background around the binder badge and it did not look that great. Any improvements, let me know.

I'll paste links or snapshots once Circle finishes on this PR.

@lesteve
Copy link
Copy Markdown
Member Author

lesteve commented Dec 19, 2017

This PR

master

@lesteve
Copy link
Copy Markdown
Member Author

lesteve commented Dec 22, 2017

Rebased to fix a conflict and screenshots added:

Master

master

This PR

pr

@lesteve
Copy link
Copy Markdown
Member Author

lesteve commented Dec 22, 2017

@choldgraf any objection about this visual tweak?

@choldgraf
Copy link
Copy Markdown
Contributor

I am definitely +1 on this PR, though in the link I can't get all three to line up since the buttons don't scale w/ the size of the page:

image

I think this problem is less-evident when there are 2 buttons rather than 3.

That said, it sounds like this issue is more one of dynamic scaling rather than one of adding in the binder button, so probably shouldn't be blocking on this PR, no? What do you think?

@lesteve
Copy link
Copy Markdown
Member Author

lesteve commented Dec 22, 2017

That said, it sounds like this issue is more one of dynamic scaling rather than one of adding in the binder button, so probably shouldn't be blocking on this PR, no? What do you think?

I am not an expert on web stuff but I think the behaviour when you adjust the screen size seems fine.

@choldgraf
Copy link
Copy Markdown
Contributor

choldgraf commented Dec 22, 2017

I guess I'm not making a value judgment on that behavior, just noting that on my 14" laptop screen (probably well within the expected browser sizes of our users) the above image is what comes up.

@lesteve
Copy link
Copy Markdown
Member Author

lesteve commented Dec 22, 2017

I guess I'm not making a value judgment on that behavior, just noting that on my 14" laptop screen (probably well within the expected browser sizes of our users) the above image is what comes up.

Interesting I can reduce the window to about half the size on my screen (14" laptop screen too) and still see the three buttons on the same row.

In any case I think that even in this case (three buttons not fitting on the screen width) having the three buttons on the same footing makes it look better than before.

@choldgraf
Copy link
Copy Markdown
Contributor

huh, that is weird! well, either way I like the change, so I'll merge in later today unless other folks chime in w/ dissenting opinions!

@GaelVaroquaux
Copy link
Copy Markdown
Contributor

GaelVaroquaux commented Dec 22, 2017 via email

@GaelVaroquaux
Copy link
Copy Markdown
Contributor

GaelVaroquaux commented Dec 22, 2017 via email

@choldgraf
Copy link
Copy Markdown
Contributor

  • a 3 column layout switching to a 1 column layout when the width isn't large enough for three columns

makes sense to me. though I am unsure as to the magical CSS incantation to make this happen

@GaelVaroquaux
Copy link
Copy Markdown
Contributor

You need the "@media" selector. I am in a meeting right now. Can't code.

@choldgraf
Copy link
Copy Markdown
Contributor

hadn't seen that before, very cool!

https://www.w3schools.com/cssref/tryit.asp?filename=trycss3_media_example1

The question is: at what point does the strange stacking happen. I suppose this depends on a combination of resolution + width, but it's weird that @lesteve and I have the same size monitor but 2 different behaviors

@larsoner
Copy link
Copy Markdown
Contributor

larsoner commented Dec 22, 2017 via email

@lesteve
Copy link
Copy Markdown
Member Author

lesteve commented Dec 22, 2017

Hmmm I can actually reproduce the behaviour from #329 (comment) on Chromium, so it looks like there is something fishy going on.

@Titan-C
Copy link
Copy Markdown
Member

Titan-C commented Dec 22, 2017

I want to add that we should use the https address for the binder badge. I have firefox57 and it does not load the image because the website is https(in github-pages and circle-ci), thus it blocks loading http resources.

@choldgraf
Copy link
Copy Markdown
Contributor

Should this PR be blocked on the CSS three-column to one-column transition problem?

Also I agree we should use HTTPS for the badge...I'll open a PR for this, good spot @Titan-C

@lesteve lesteve force-pushed the align-binder-badge branch from ee1d94c to 1db9605 Compare January 2, 2018 09:11
@lesteve
Copy link
Copy Markdown
Member Author

lesteve commented Jan 2, 2018

I arranged the download buttons and the binder badge vertically. I feel this is a lot simpler and more robust than trying to put them on the same row and making it work with media queries depending on the screen width (The sphinx read the docs theme already has some media query that decides to display the side-bar which further complicates matters. Other themes may have similar quirks as well).

This is how it looks:
screenshot-2018-1-2 seaborn example sphinx-gallery 0 1 13-git documentation.

Copy link
Copy Markdown
Contributor

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Apparently we already do something like this in MNE with CSS for the sphx-glr-download, +1 from me for making it the default for that and binder

@choldgraf
Copy link
Copy Markdown
Contributor

I'm fine w/ this as well

@lesteve
Copy link
Copy Markdown
Member Author

lesteve commented Jan 5, 2018

@Titan-C and/or @GaelVaroquaux any objection to this one?

@Titan-C
Copy link
Copy Markdown
Member

Titan-C commented Jan 5, 2018

I'm fine with this. I actually prefer this stacking of the buttons, it really makes the layout more uniform. +1

@lesteve
Copy link
Copy Markdown
Member Author

lesteve commented Jan 8, 2018

OK three people agree, so I am going to merge this one. I guess this can always be modified with some custom CSS if needed.

@GaelVaroquaux
Copy link
Copy Markdown
Contributor

In general, +1, though on wide screens it creates a bit of a waste of screen real estate.

The best design would be to have it floating on the side for wide pages, something like (in addition and separate from the other CSS instructions to sphx-glr-footer):

div.sphx-glr-footer @media only screen and (min-width: 100ex) {
    position: fixed;
    right: 10px;
    top: 80px;
    background: #f2f2f2;
    padding: 1ex;
    border: solid 1px #ccc;
}

I put the 100ex in as a ballpark estimate, maybe something larger is required to avoid overlap with text on too narrow screens. This can be tuned by seeing what happens when the window of the browser is resized.

The benefit is that it would make the binder and download buttons much more visible. Right now, my experience is that users often miss them.

@lesteve lesteve merged commit 77f14c2 into sphinx-gallery:master Jan 8, 2018
@lesteve lesteve deleted the align-binder-badge branch January 8, 2018 12:49
@lesteve
Copy link
Copy Markdown
Member Author

lesteve commented Jan 8, 2018

Right now, my experience is that users often miss them.

I agree with this, a notebook-like example like this encourages users to copy and paste cell by cell from the HTML rather than go to the bottom of the page. This may lead to confusion as in scikit-learn/scikit-learn#10394, where the plt.show is right at the end of the example.

IMO it would be nice to put:

  • either a link before the example than goes to the download section at the bottom
  • the download buttons before the example. Probably this this takes too much space, so better suggestions welcome!

@GaelVaroquaux
Copy link
Copy Markdown
Contributor

GaelVaroquaux commented Jan 8, 2018 via email

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