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

[core] Support for bridge maintainers' donation URLs #2102

Merged
merged 5 commits into from
Jun 25, 2021

Conversation

Bockiii
Copy link
Contributor

@Bockiii Bockiii commented May 4, 2021

Contribution to #2099

This adds a new const "DONATION_URL" to the bridges. If set, a new (green for better distinguishing) button will be shown on the HTML overview, linking to that URL.

@Bockiii Bockiii mentioned this pull request May 4, 2021
9 tasks
@Bockiii
Copy link
Contributor Author

Bockiii commented May 4, 2021

If you want to test this, just add something like this to your bridge:

const MAINTAINER = 'Me';
const NAME = 'My Bridge';
const URI = 'https://www.google.com';
const DONATION_URI = 'https://github.com/em92/rss-bridge/blob/donation/doc/donate/em92.md';
const CACHE_TIMEOUT = 21600; // 6h
const DESCRIPTION = 'Yay, google.';

@em92
Copy link
Contributor

em92 commented May 4, 2021

Not sure about design.
Снимок экрана_2021-05-05_01-01-15

@ORelio and @JackNUMBER hi! Any suggestions about that link?

Also ping @somini @VerifiedJoseph @sysadminstory @corenting @AntoineTurmel @teromene @mitsukarenai @logmanoriginal. If you haven't read discussion about donation, you can read it here #2063

@em92
Copy link
Contributor

em92 commented May 4, 2021

Forgot to ping @Roliga

@Bockiii
Copy link
Contributor Author

Bockiii commented May 4, 2021

I'm pretty sure that is your browser.
Firefox:
image

Chrome:
image

Edge:
image

It doesn't matter how large or small I make the window, the button doesn't morph like in your screenshot

@Bockiii
Copy link
Contributor Author

Bockiii commented May 4, 2021

About the colors: I'm not the best person for color choices, so feel free to change those :) My idea was to use a darker green for the link and a lighter green for the hover. If you have better values, go for it. I just wanted to use a positive color so the user notices it.

@JackNUMBER
Copy link
Contributor

Thank you @Bockiii for initiating the design update.
About the

I suggest this color, as a tribute to the old logo ;)
image

Or something less "eye catchy". This way the buttons are limited to the output format.
image

@JackNUMBER
Copy link
Contributor

JackNUMBER commented May 4, 2021

About the 2-lines-button issue :

Fix 1

It can be fixed with min-height: 40px, reduce Y's paddings and remove line-height.
image <- desktop
image <- mobile

Fix 2

In an other hand, if we want to keep the same design everywhere we can apply white-space: nowrap, min-width: 200px and remove line-height but it can bring side effects on mobile.
image <- desktop
image <- mobile

@Bockiii
Copy link
Contributor Author

Bockiii commented May 4, 2021

Thank you @Bockiii for initiating the design update.

No problem :)

I suggest this color, as a tribute to the old logo ;)

That color is also fine with me. If you have an idea for the lighter "hover" color, post the values and I can change them

Or something less "eye catchy". This way the buttons are limited to the output format.

I actually want it to be catchy. If the maintainer doesnt provide the URL, it isn't shown at all, so it will only be shown on bridges with donation info. That way, people will actually notice the option and can then still choose to ignore it.
The eye-catchiness was the main reason I added it as an additional button in the location that everyone looks at anyways.

@Bockiii
Copy link
Contributor Author

Bockiii commented May 4, 2021

@JackNUMBER I like Fix 1 better than Fix 2.

Also, "Fix 3" could be just to write "Donate" on the button and not have the issue at all :D

I just wanted to make clear that you donate to the bridge maintainer and not to rss-bridge in general (we could/should put a button for that somewhere as well btw)

@JackNUMBER
Copy link
Contributor

JackNUMBER commented May 4, 2021

@Bockiii

I actually want it to be catchy. If the maintainer doesnt provide the URL, it isn't shown at all, so it will only be shown on bridges with donation info. That way, people will actually notice the option and can then still choose to ignore it.
The eye-catchiness was the main reason I added it as an additional button in the location that everyone looks at anyways.

Agreed!
Default: #ff6600
Hover: #ff8a3b

I just wanted to make clear that you donate to the bridge maintainer and not to rss-bridge in general (we could/should put a button for that somewhere as well btw)

Agreed too! That's why I keep the complete label you did :)

@Bockiii
Copy link
Contributor Author

Bockiii commented May 4, 2021

donezo

Copy link
Contributor

@JackNUMBER JackNUMBER left a comment

Choose a reason for hiding this comment

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

More code style comments than functional 😄

formats/HtmlFormat.php Outdated Show resolved Hide resolved
formats/HtmlFormat.php Outdated Show resolved Hide resolved
formats/HtmlFormat.php Outdated Show resolved Hide resolved
static/HtmlFormat.css Outdated Show resolved Hide resolved
lib/FormatAbstract.php Outdated Show resolved Hide resolved
actions/ListAction.php Outdated Show resolved Hide resolved
actions/DisplayAction.php Outdated Show resolved Hide resolved
@Bockiii
Copy link
Contributor Author

Bockiii commented May 5, 2021

Woops. didnt mean to start the review :D You can close that.

I added a new commit, doing all the style changes from @JackNUMBER

actions/DisplayAction.php Outdated Show resolved Hide resolved
@Bockiii
Copy link
Contributor Author

Bockiii commented May 5, 2021

@em92 Do you want to add a donate button to rss-reader as well?

I would suggest doing it next to the hide-inactive-button. This way it's out of sight usually but still visible enough to not completely overlook. I would do it in the same visual style as the maintainer donate buttons, just smaller to fit the hide-inactive button.

Thoughts?

Also, do we want to make this configurable? So add a setting to not show donation buttons at all? I'm 80% "no" on this, but maybe someone else has a good point to make.

@teromene
Copy link
Member

teromene commented May 5, 2021

The design looks fine to me, and I think that the idea of allowing donations for bridge maintainers is a great idea as well.

@JackNUMBER
Copy link
Contributor

Can I suggest something like that?
image

@Bockiii
Copy link
Contributor Author

Bockiii commented May 5, 2021

I am more on the side of "Make it visible on every specific bridge and less visible on the front page". So my vote is against the small donate link.

You can inline-review and add changes btw @JackNUMBER . If you change the .css to the 2-line-fix1, I can accept them here in this PR.

@somini
Copy link
Contributor

somini commented May 6, 2021

image

I like this, and would propose combining this with a project-wide banner on top asking for donations. Sort of like Wikipedia?

@Bockiii
Copy link
Contributor Author

Bockiii commented May 6, 2021

Isn't that one of the most universally hated banners in the web?

I'm still pro-button, but if you want to go with the text link, I would definitely not do a banner.

I'll just say that this will be overlooked completely.

@Bockiii
Copy link
Contributor Author

Bockiii commented May 6, 2021

Should we run a public poll for the different solutions or wo wants to decide this? I can code the solution, I just need to know the "what" :D

@em92
Copy link
Contributor

em92 commented May 6, 2021

@Bockiii, I think we need to have both "clearly visible button in html preview" and "small text link in bridge card" and option, where instance admin chooses one (or even none, if admin does not want to show them). The default one should be "clearly visible button", 'cos of strategy to promote donation.

would propose combining this with a project-wide banner on top asking for donations. Sort of like Wikipedia?

@somini , that is discussable, but not here.

@em92 em92 changed the title [Core] Support for donation URLs [core] Support for bridge maintainers' donation URLs May 6, 2021
@em92
Copy link
Contributor

em92 commented May 6, 2021

Do you want to add a donate button to rss-reader as well?

If you mean add donation link to feed items, then no. That is definitely going to be frustrating for users.

@Bockiii
Copy link
Contributor Author

Bockiii commented May 6, 2021

I meant a "donate to rss-bridge" link somewhere. One option would be next to the "show inactive" button. At the end of the site and usually half-greyed out if you dont hover over it. As I user, i would be find with it being there.

So I will do this:

  • One button like it is now on the html overview
  • One link on the bridgecard like in sominis screenshot
  • Global opt-out to not display donation links at all. Placed in the config.ini

I would not do it opt-in since we want to promote the usage. In the future, we could switch the default in the config ini to opt-in.

@Bockiii
Copy link
Contributor Author

Bockiii commented May 6, 2021

Current state:

  • Config option to disable donations. Default is "true" for donation display
  • Bridge Maintainer Donations as button in HTML and as link in bridgelist view
  • RSS-Bridge general donation link (I need an actual link to point to @em92 as I couldn't find anything) in the footer of the page.

html view:
image

bridgelist view:
image

general rss-bridge dono:
image

Thoughts?

@ORelio
Copy link
Contributor

ORelio commented May 6, 2021

Thoughts?

Hi, just read all the discussion and I totally agree with this end result. With donate link not too agressive in bridge list, more visible button in bridge details, and setting to optionally disable the feature 👍

@ORelio
Copy link
Contributor

ORelio commented May 6, 2021

However I'm not super confident in the financial part of the feature: what happens if several people work on a bridge? How to split/manage donations for the project-wide donation link? I'm not planning to accept donations for my own bridges in the near future but I'm still curious 🤔

@Bockiii
Copy link
Contributor Author

Bockiii commented May 6, 2021

for the general: no idea.

For the maintainer: I think the majority of bridges have 1 maintainer. If its more than one, I assume the team has to decide if the "old" maintainer is still worthy enough.

We could also do a mandatory site in the repo were we list a kind-of-history. "Originally by Peter, maintained by Paul since 2018. Link for Peter <> Link for Paul <>" or something... idk. I just picked up the todo, I have no game in this fight

@somini
Copy link
Contributor

somini commented May 7, 2021

Thoughts?

LGTM.

@Bockiii
Copy link
Contributor Author

Bockiii commented May 11, 2021

it's been 4 days. Anyone still thinking about anything?

@Bockiii
Copy link
Contributor Author

Bockiii commented May 11, 2021

Ah, I see that the discussion in the discussions post is still going. All good then.

@teromene
Copy link
Member

teromene commented Jun 8, 2021

Any blocker once PHPCS problems are fixed ? @em92

@Bockiii
Copy link
Contributor Author

Bockiii commented Jun 8, 2021

@em92 @teromene all fixes fixed.

@@ -178,10 +179,14 @@ private static function getFooter($totalBridges, $totalActiveBridges, $showInact

}

if($donationsAllowed) {
$donation = ' ~ <a href="http://www.google.com">Donate</a>';
Copy link
Contributor

Choose a reason for hiding this comment

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

Wnat is expected here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thats right. Thats the rss-bridge-general-donation-url, so not bridge specific but for rss-bridge itself. You need to define that link

@Bockiii
Copy link
Contributor Author

Bockiii commented Jun 20, 2021

Okay. If you ever want to add a general dono URL, you have an example.

Other than that, we would also need to adapt documentation. How far along is the rework?

@em92
Copy link
Contributor

em92 commented Jun 25, 2021

Other than that, we would also need to adapt documentation. How far along is the rework?

Mark Ruffallo and Leonardo Di Caprio meme

@em92 em92 merged commit ecaae73 into RSS-Bridge:master Jun 25, 2021
@em92
Copy link
Contributor

em92 commented Jun 25, 2021

gj @Bockiii !

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.

6 participants