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 the styling of the donation buttons #169

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

meenbeese
Copy link
Collaborator

@meenbeese meenbeese commented Sep 16, 2023

Closes #150

Preview link: https://934d6be4.organicmaps-github-io.pages.dev/
(I do not know why some changes are not showing up correctly.)

  • Add a green colored border on mouse hover
    to display our accent color on the icon border
  • Make the donation icons bigger and adjust
    how many are shown based on the screen size
  • Remove duplicated donation icons at the end
Desktop: Before Desktop: After
desk1 desk2
Mobile: Before Mobile: After
mob1 mob2

@meenbeese meenbeese force-pushed the redo-donate-buttons branch 8 times, most recently from 58343bb to 78ece3e Compare September 16, 2023 20:48
@meenbeese meenbeese temporarily deployed to production September 16, 2023 23:59 — with GitHub Actions Inactive
Copy link
Member

@biodranik biodranik left a comment

Choose a reason for hiding this comment

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

Thanks! Did you try to set up Cloudflare Preview deployments in your forked repo? They, unfortunately, don't allow to preview PRs from contributor's forks.
Or do you know other ways to see a live preview for contributor's PRs?

content/news/2022-12-23/122/index.ru.md Outdated Show resolved Hide resolved
@meenbeese
Copy link
Collaborator Author

Thanks! Did you try to set up Cloudflare Preview deployments in your forked repo? They, unfortunately, don't allow to preview PRs from contributor's forks. Or do you know other ways to see a live preview for contributor's PRs?

This is really weird. I tried to set up a preview website using two different providers and both of them are not showing the changes I made to the icons, only to the text. I don't know why this is the case since zola serve works perfectly on my machine.

Due to this, I would recommend you to clone the redo-donate-buttons branch from my fork repo and just build it locally. This looks like the best way as other methods are not reliable for some reason. I'm sorry for the inconvenience caused.

@biodranik
Copy link
Member

This is really weird. I tried to set up a preview website using two different providers and both of them are not showing the changes I made to the icons, only to the text. I don't know why this is the case since zola serve works perfectly on my machine.

Can you please share links to previews? Did you do zola build locally and compare the HTML output with the expected one?

@meenbeese meenbeese temporarily deployed to production September 17, 2023 05:35 — with GitHub Actions Inactive
@rtsisyk
Copy link
Member

rtsisyk commented Sep 17, 2023

I like the screenshots. Please fix translations separately to reduce the size of PR.

@meenbeese
Copy link
Collaborator Author

Can you please share links to previews?

Here is the Cloudflare preview but do note that it does not reflect all my changes correctly: https://a5f2e489.organicmaps-github-io.pages.dev/donate/

Did you do zola build locally and compare the HTML output with the expected one?

I don't know what you mean exactly by that. I ran both zola build and zola serve again now and it matches what I expect it to be.

@meenbeese meenbeese changed the title Fix donation buttons' styling and some rewording Fix the styling of the donation buttons Sep 17, 2023
@meenbeese meenbeese force-pushed the redo-donate-buttons branch 3 times, most recently from 27bdc1f to 9d5a5f8 Compare September 17, 2023 20:23
@meenbeese
Copy link
Collaborator Author

meenbeese commented Sep 17, 2023

I like the screenshots. Please fix translations separately to reduce the size of PR.

Done! Here is the new PR for that: #173

Copy link
Member

@biodranik biodranik left a comment

Choose a reason for hiding this comment

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

  1. Can you please rebase this commit on the latest master branch and force-push it here again?
  2. The preview link didn't show any changes to buttons for some reason. What is the branch-preview link?

@meenbeese meenbeese temporarily deployed to production September 19, 2023 22:22 — with GitHub Actions Inactive
@meenbeese
Copy link
Collaborator Author

meenbeese commented Sep 19, 2023

  1. Can you please rebase this commit on the latest master branch and force-push it here again?

Done! As you recommended, git rebase is much cleaner. Thanks.

  1. The preview link didn't show any changes to buttons for some reason. What is the branch-preview link?

I couldn't manage to get a preview link for this branch. I you could give me some level of access as discussed earlier, that would be possible perhaps. Did you try to build it in your local machine to test it? That's a good way as well.

Copy link
Member

@biodranik biodranik left a comment

Choose a reason for hiding this comment

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

  1. How does it look on mobile screen? The current version fits it well. Most of our website audience comes from mobiles.
  2. Now buttons take too much space. Is it really necessary? Maybe leaving them more compact has its advantages?
  3. It is not clear for some users that these icons are actually clickable. Does it make sense to show some light border (aka button) on them, and highlight it on mouse hover/finger touch?
  4. As it was mentioned in Migrate all translations to Weblate #164, some translations are not migrated to weblate and are not supported yet by tools/i18n script. Can you please help to finish this migration? Then it would be easy to add/modify text blocks everywhere, and keep everything in sync. There should be an issue or a closed PR from @rtsisyk about that (and he can also suggest how to finish it).


Klik op u voorkeurbetaalmetode-ikoon hier onder:

{{ donate_buttons() }}
Copy link
Member

Choose a reason for hiding this comment

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

Please check the English version (and a few other langs that are not yet synchronized with Weblate). We display donate buttons twice:

  1. If someone already wants to donate without reading the details, the upper donate block is immediately visible on a mobile phone screen.
  2. If someone is not sure, then there is another donate block below, after reading the motivating text (it will be improved later).

Removal of the second block may not be convenient, think of mobile users first.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. I checked and updated all the missing translations manually. I will try to make them sync with Weblate as discussed.
  2. The text at the top is not huge and it won't prevent anyone from seeing the donation buttons immediately.
  3. Repetitive donations buttons are not good UX imo, because it looks too forceful and insisting instead of convincing.

@meenbeese
Copy link
Collaborator Author

  1. How does it look on mobile screen? The current version fits it well. Most of our website audience comes from mobiles.

The mobile screenshots are also available in the top comment. If I left something important out, let me know please.

  1. Now buttons take too much space. Is it really necessary? Maybe leaving them more compact has its advantages?

Yeah, this may be true for the mobile version. I can adjust that even though screen space is not a huge issue and making them too compact makes them harder to click imo.

  1. It is not clear for some users that these icons are actually clickable. Does it make sense to show some light border (aka button) on them, and highlight it on mouse hover/finger touch?

I already implemented a dark green border on mouse hover/touch, and I can expand this if you think that is not enough. I will try to work on this more as I haven't done a great job about this distinction.

  1. As it was mentioned in Migrate all translations to Weblate #164, some translations are not migrated to weblate and are not supported yet by tools/i18n script. Can you please help to finish this migration? Then it would be easy to add/modify text blocks everywhere, and keep everything in sync. There should be an issue or a closed PR from @rtsisyk about that (and he can also suggest how to finish it).

I will try to investigate this then, as I am not very familiar with the site's inner workings. Can you help me guide towards a solid initial implementation @rtsisyk ?

@biodranik
Copy link
Member

It would be awesome to start with porting missing translations to weblate. They are described in more details here: https://github.com/organicmaps/organicmaps.github.io/blob/master/TRANSLATIONS.md

@meenbeese
Copy link
Collaborator Author

It would be awesome to start with porting missing translations to weblate.

Can't that be done in another PR where we only focus on translations? I can create a separate issue for that after this is done.

Copy link
Member

@biodranik biodranik left a comment

Choose a reason for hiding this comment

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

Arent these files regenerated from weblate? Shouldn't you also edit/change .po files?

I've mentioned finishing weblate translation because now some files are translated using .po, and some are not.

@meenbeese meenbeese force-pushed the redo-donate-buttons branch from 9e4a5ca to 620a15c Compare November 2, 2023 01:07
@meenbeese meenbeese temporarily deployed to production November 2, 2023 01:07 — with GitHub Actions Inactive
@meenbeese meenbeese temporarily deployed to production November 2, 2023 01:53 — with GitHub Actions Inactive
@meenbeese meenbeese temporarily deployed to production November 2, 2023 01:55 — with GitHub Actions Inactive
@meenbeese meenbeese force-pushed the redo-donate-buttons branch from ece4ddf to 05c19c6 Compare November 2, 2023 01:57
Signed-off-by: Roman Tsisyk <[email protected]>
Signed-off-by: meenbeese <[email protected]>

Fix donation buttons styling

Signed-off-by: meenbeese <[email protected]>

Revert changes related to the wording

Signed-off-by: meenbeese <[email protected]>

tools/i18n.sh

Signed-off-by: Alexander Borsuk <[email protected]>

Regenerated

+ Fixed links in Arabic

Signed-off-by: Roman Tsisyk <[email protected]>
Signed-off-by: meenbeese <[email protected]>

Reorganize and unify the donate page

Fix donation buttons styling

Signed-off-by: meenbeese <[email protected]>

Revert changes related to the wording

Signed-off-by: meenbeese <[email protected]>

tools/i18n.sh

Signed-off-by: Alexander Borsuk <[email protected]>

Regenerated

+ Fixed links in Arabic

Signed-off-by: Roman Tsisyk <[email protected]>
Signed-off-by: meenbeese <[email protected]>

Reorganize and unify the donate page
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.

Make donation buttons more obvious
3 participants