-
Notifications
You must be signed in to change notification settings - Fork 33
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
base: master
Are you sure you want to change the base?
Conversation
58343bb
to
78ece3e
Compare
There was a problem hiding this 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?
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 Due to this, I would recommend you to clone the |
02340f2
to
9b7a33d
Compare
Can you please share links to previews? Did you do |
I like the screenshots. Please fix translations separately to reduce the size of PR. |
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/
I don't know what you mean exactly by that. I ran both |
27bdc1f
to
9d5a5f8
Compare
Done! Here is the new PR for that: #173 |
9d5a5f8
to
51732a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Can you please rebase this commit on the latest master branch and force-push it here again?
- The preview link didn't show any changes to buttons for some reason. What is the branch-preview link?
51732a5
to
9e4a5ca
Compare
Done! As you recommended,
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- How does it look on mobile screen? The current version fits it well. Most of our website audience comes from mobiles.
- Now buttons take too much space. Is it really necessary? Maybe leaving them more compact has its advantages?
- 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?
- 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() }} |
There was a problem hiding this comment.
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:
- If someone already wants to donate without reading the details, the upper donate block is immediately visible on a mobile phone screen.
- 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I checked and updated all the missing translations manually. I will try to make them sync with Weblate as discussed.
- The text at the top is not huge and it won't prevent anyone from seeing the donation buttons immediately.
- Repetitive donations buttons are not good UX imo, because it looks too forceful and insisting instead of convincing.
The mobile screenshots are also available in the top comment. If I left something important out, let me know please.
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.
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.
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 ? |
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 |
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. |
There was a problem hiding this 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.
9e4a5ca
to
620a15c
Compare
ece4ddf
to
05c19c6
Compare
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
05c19c6
to
d5b0f29
Compare
Closes #150
Preview link: https://934d6be4.organicmaps-github-io.pages.dev/
(I do not know why some changes are not showing up correctly.)
to display our accent color on the icon border
how many are shown based on the screen size