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

Add donate_url and opencollective options to add donate buttons to the sidebar #143

Closed

Conversation

sloria
Copy link
Contributor

@sloria sloria commented Sep 6, 2018

When either are set, adds a "Donate" section to the sidebar.
If donate_url is set, a "donate" shield button is used.
If opencollective is set, an OpenCollective button is used.

With donate_url = ""https://www.paypal.me/StevenLoria":

donate button

With opencollective = "marshmallow":

opencollective button

Both may be set:

both buttons

I also removed the underlines from badges in the sidebar, which I assumed were inadvertent:

Before

with underlines

After

without underlines

When either are set, adds a "Donate" section to the sidebar.
If `donate_url` is set, a "donate" shield button is used.
If `opencollective` is set, an OpenCollective button is used.
@sloria sloria changed the title Donate url and opencollective Add donate_url and opencollective options to add donate buttons to the sidebar Sep 6, 2018
@sloria
Copy link
Contributor Author

sloria commented Sep 6, 2018

Some notes:

@bitprophet bitprophet added this to the 0.7.12 milestone Sep 6, 2018
@bitprophet
Copy link
Collaborator

Links: #132, #128 (presumably).

The collective homepage more clearly shows the
different ways to donate (tiers, sponsorships)
@sloria
Copy link
Contributor Author

sloria commented Sep 6, 2018

Ah, yes. Sorry, I should have looked at those existing issues/PRs. I believe this should close #132 (but not #128). Support for other donation button types (e.g. for Patreon, Stripe, etc.) can always be added in the future.

@bitprophet
Copy link
Collaborator

bitprophet commented Oct 2, 2018

Pondering whether we should try to generalize the OpenCollective button so we're not locked into adding a smidge of custom HTML for every single donation service out there (see #132 for a very short list of obvious candidates, for example).

On the one hand, yay, more user control!

On the other hand, jeez, isn't this why Sphinx has the html_sidebars option to begin with? Why wouldn't we just ask users to put their own HTML in here?

On the other other hand, asking users to write their own "custom" HTML that's actually the same for every (OpenCollective|Patreon|etc) user is silly, if we assume we'll have more than a few users of each service.

Main downside of generifying it is losing things like the OpenCollective's width=300 (though I question why that's there to begin with - I don't see anything else in our styling that implies 300 is the right number there?) - though once again, users who need that level of control have the "write their own sidebar snippet" exit hatch...

(The OpenCollective button color thing, on the other hand, just falls into "supply your own URL".)

@bitprophet
Copy link
Collaborator

Part of that is whether the Sphinx templating handles iterables well, I can't recall. If it doesn't, then I guess we're stuck with "one snippet per service" and whatever.

@bitprophet
Copy link
Collaborator

OK, INI syntax doesn't seem to do lists. We could make it a conf.py level setting, but I think at this point I don't care enough. If donation.html gets too heavy we can always opt to generify it later, perhaps. Though that's harder.

@bitprophet
Copy link
Collaborator

bitprophet commented Oct 3, 2018

Grumble, one issue with the "handful of orthogonal donation buttons" approach is it'll make the hide/show {% if %} statement get completely ridiculous. Guess we could at least move that logic to the top of alabaster.css_t or something like we do with some other things, but still.

Actually...if we avoid the (admittedly, would be nice) flavor text and header at top/bottom - in other words, making donation.html just a loose collection of independently shown/hidden elements - that solves the problem neatly. It's also just kinda how the sidebar works overall right now.

I don't like that fact, but I'm willing to punt to a general overhaul of the sidebar sometime later.

EDIT: More grumbling...right, we may still want some H3 to separate donation elements from eg Tidelift, if both are enabled; and we need to handle the case of "OpenCollective but no generic donation button" etc. OTOH, if we put Tidelift at the bottom, then it's largely just a series of buttons above it, which isn't the worst thing ever (again see punting about a general overhaul of sidebar organization/styling). I'll play around.

@bitprophet
Copy link
Collaborator

bitprophet commented Oct 3, 2018

Hrm, the screenshot from the OP doesn't match what I am seeing re: multiple donate buttons in play, there's zero padding between them in my case:

image

I'm pulling in Steven's code piecemeal but I double checked and I don't seem to be missing any styling or elements 🤔

EDIT: if I wrap the individual bits in <p> tags (which eg the Tidelift stuff was also wrapped in) it's a lot nicer. Steven's code had this but only around the both of them, not in between. Shrug?

Also, I'm realizing, we can probably get away with putting a setting-agnostic h3 in here, because users should only have this enabled if they're using at least one of the options within...? Oh, no...our install instructions say to enable donation.html in the default list. Except our config docs all say "you need to make sure you enable donation.html" too. This whole thing needs a damn overhaul 😞

Anyway, yea, gotta go back to the Big Dumb If Statement for that I think, otherwise users who don't care and blindly followed the install instructions will accidentally upgrade & find a nice 'Donation' header floating around in their sidebar.

bitprophet added a commit that referenced this pull request Oct 3, 2018
bitprophet added a commit that referenced this pull request Oct 3, 2018
With a bunch of custom changes, see stream of consciousness
commentary in #143.
@bitprophet
Copy link
Collaborator

OK this should be done, with the abovementioned customizations, will release soonish.

@bitprophet bitprophet closed this Oct 3, 2018
@bitprophet
Copy link
Collaborator

Huge grumble, on RTD this all looks a bit weird and I have to assume it's to do with their custom modifications.

On my localhost:
image

On RTD:
image

Notice how the H3 is now bumping back up against the search bar! When using Chrome's inspector, it seems like one or the other is now floating/clearing incorrectly - hovering on the H3 shows its top-margin appearing above the search bar, instead of between the search bar and the H3's text.

Only thing I can see in RTD's custom CSS is this snippet, though the inspector doesn't appear to reflect it (and it would presumably apply to the Tidelift paragraph element, not the H3, in either case, as ethical-alabaster is the little ethical sponsored ad):

.ethical-alabaster::before {
    /* Alabaster's search box above the ad is floating */
    clear: both;
    content: '';
    display: table;
    margin-top: 3em;
}

Gonna try disabling those ads for Alabaster anyways (I do pay the RTD folks monthly for their awesomeness, even though both parties use the others' software!) to see if that confirms the theory. If I can remember how.

@bitprophet
Copy link
Collaborator

Nope, that's not it (check off "don't show ads" under Advertising in RTD admin), but partly because the element is still there, just empty. But I am seeing the ::before pseudo-element in the source view, so...hrm.

Same issue in Safari so it's not just Chrome.

Gonna have to punt on this for now tho because I've already spent so much time here today & it doesn't actually look that "off" unless you knew how it looked on my localhost. Nor is it any worse than the rest of the messed up spacing in the sidebar as a whole (see above re: wanting to overhaul the whole thing).

Still tho. Computers. 😡

div.sphinxsidebar .badge:hover {
border-bottom: none;
}

Copy link

@jab jab Oct 6, 2018

Choose a reason for hiding this comment

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

I think this whole commit (9c1fdac) is just working around the root cause, which I fixed in #125 almost a year ago but which still needs to be merged.

(I think the root cause is that some CSS in the theme that intends to clear the bottom border for all images is accidentally getting overridden by some CSS that comes after it.)

#125 has gotten 👍, but still has not been merged. Once it is merged, I think this workaround can be removed.

/cc @bitprophet @sloria

Copy link

Choose a reason for hiding this comment

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

Hi @bitprophet, my previous comment is still the case, so I just thought I'd see if you might have a moment to merge my PR. Thanks for taking a look if you can!

Copy link

Choose a reason for hiding this comment

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

(#125)

@sloria sloria deleted the donate-url-and-opencollective branch October 11, 2018 00:31
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

3 participants