fix: donation banner tweaks #7703

sultanowski · 2022-11-15T21:31:18Z

What

Replace checkbox and text to [X] button.
Better display on both mobile and desktop.
Refactor of document.cookie code and placed it directly in donate_banner.tt.html file.
Set the banner cookie time to 14 days.

Screenshot

https://imgur.com/a/3KPz3Ov

Related issue(s) and discussion

…tored code.

alexgarel

@sultanowski really cool !

I tested it, it works for me.

I think there is code in Display.pm (line 7621) that should be cleaned up.

I wait for some suggestion resolutions to approve.

alexgarel · 2022-11-16T09:40:59Z

templates/web/common/includes/donate_banner.tt.html

+    d.setTime(d.getTime() + (bcexdays*60*60*24*1000));
+    let expires = 'expires=' + d.toUTCString();
+    document.cookie = bcname + '=' + bcval + ';' + expires + ';path=/';
+    console.log(expires);


we should remove this :-)

Suggested change

console.log(expires);

Oops, forgot about those! Thank you, I will remove it soon, also unused vars and logs.

alexgarel · 2022-11-16T09:42:37Z

templates/web/common/includes/donate_banner.tt.html

+  function deleteBannerCookie(bcname) {
+    d.setTime(d.getTime() + (60*60*24*1000));
+    let expires = 'expires=' + d.toUTCString();
+    document.cookie = bcname + '=;' + expires + ';path=/';
+  }


Why do we need this, and not only call setBannerCookie ?

It's like a double-check for me - cleans the cookie value and set expire for current time, if you feel like it is not really important I can test the code without it.

ok, it's ok if we keep it, but maybe a comment to explain is good :-)

alexgarel · 2022-11-16T10:09:59Z

templates/web/common/includes/donate_banner.tt.html

+
+<script>
+  let d = new Date();
+  let expires = 'expires=' + d.toUTCString();


it seems we don't need this any-more ?

alexgarel · 2022-11-16T10:11:32Z

templates/web/common/includes/donate_banner.tt.html

+  let d = new Date();
+  let expires = 'expires=' + d.toUTCString();
+  let bannerID = document.getElementById('donate-banner');
+  let bcval = "1";


it seems we don't need this any-more ?

alexgarel · 2022-11-16T10:15:22Z

templates/web/common/includes/donate_banner.tt.html

+
+  function DonationButton() {
+    deleteBannerCookie('off-donation-banner');
+    setBannerCookie('off-donation-banner', 1, 14);


I seems to me 14 days is not enough, it's a bit pushy to users.

If I donate, I would expect this not to show up again for at least 180 days !

If possible, I would set it on whole openfoodfacts.xx subdomain (not only for eg. world) (that is you can set domain to the last two components of domain).

@teolemon what is your though ?

stephanegigandet · 2022-11-16T11:38:24Z

On mobile, the banner is now more a full screen banner than a banner:

after:

before:

And if we remove the message to close the box, and put the cross instead we would get a smaller banner:

stephanegigandet · 2022-11-16T11:43:59Z

And on wide screen, we lose the max width that we had before, which makes the text hard to read on a very long line.

after:

before:

It's certainly a personal test, but I prefer the behaviour we had before for mobile and desktop.

Would it be possible to have a behaviour more like we had before, but with the new "Close" mark you added?

stephanegigandet · 2022-11-16T11:44:55Z

Note that yesterday we added a 2nd donation box (identical except that it cannot be closed) at the bottom of the page, so unfortunately it's causing some conflicts.

sultanowski · 2022-11-16T12:20:16Z

Pushed cosmetic changes mentioned in this discussion, banner is slightly smaller than the actual one. What should I do to resolve this conflict? I didn't pulled to this branch during development and I don't want to merge it with main - that might be even worse I think.

alexgarel · 2022-11-16T12:47:45Z

@sultanowski the best would be to sync main branch on your project, update main locally, switch to your branch, then use "rebase main" (read some tutorial, it's a good command to master). Then you have to "git push --force".

sultanowski · 2022-11-16T19:30:19Z

@alexgarel I found solution to apply a cookie for all subdomains and all possible versions (.org, .net, etc.) now I am going to rebase and update this branch.

alexgarel

Thank you !
@sultanowski still a small problem and we are done :-)

templates/web/common/includes/donate_banner.tt.html

…..facts.xxx).

sonarqubecloud · 2022-12-05T12:26:49Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

sultanowski · 2022-12-05T12:59:24Z

What I've done in small summary:

Delete donate_banner_bottom.tt.html file and create new instance of it inside donate_banner.tt.html file.
Replace long checkbox with label to graphical [x] button.
Banner's cookie should work with every type of open...facts page. Now URL is dynamically taken from client's URL and it apply on close button click.
Fix max-width of donation-banner content, so now it works like previously on wide screens.
Reduce margins and paddings to make donation banner compact in size.
Clean-up of Display.pm file and delete unused cookie jQuery function.

alexgarel · 2022-12-05T16:43:52Z

Fine @sultanowski, thank you so much for your dedication !

I think we could improve styling because the banner is a bit deported on the right because of the "hand-heart" icon on the right… but that would be for another PR :-)

fix: Added [x] button to donation banner, changed cookie JS and refac…

5affee0

…tored code.

sultanowski requested a review from a team as a code owner November 15, 2022 21:31

github-actions bot added CSS 🎁 donations Template::Toolkit The templating toolkit used by product opener. The starting point for HTML/JS/CSS fixes. labels Nov 15, 2022

github-actions bot assigned sultanowski Nov 15, 2022

sultanowski changed the title ~~fix: Added [x] button to donation banner, changed cookie JS and refac…~~ fix: Donation banner. Nov 15, 2022

sultanowski marked this pull request as draft November 15, 2022 21:35

sultanowski changed the title ~~fix: Donation banner.~~ [wip] fix: Donation banner. Nov 15, 2022

style: removed unused styles (donation banner)

2eadb40

sultanowski changed the title ~~[wip] fix: Donation banner.~~ fix: donation banner tweaks Nov 15, 2022

sultanowski marked this pull request as ready for review November 15, 2022 21:41

github-actions bot added the 💥 Merge Conflicts 💥 Merge Conflicts label Nov 16, 2022

alexgarel reviewed Nov 16, 2022

View reviewed changes

fix: Remove additional margins, set max-width, delete unused code.

5682a53

github-actions bot added the Display label Nov 16, 2022

fix: deleted console.log

14a6546

fix: Set cookie on all xx.openfoodfacts.xxx subdomains

a7d2ba2

alexgarel requested changes Nov 21, 2022

View reviewed changes

templates/web/common/includes/donate_banner.tt.html Outdated Show resolved Hide resolved

sultanowski added 4 commits November 28, 2022 14:34

refactor: Donation banner component.

1291831

Merge branch 'main' into sultanowski-donation-banner

a1484a5

fix: apply cookie for every possible version of the project (xx.open.…

204edea

…..facts.xxx).

rebase: picked newest changes

e4d5cd9

github-actions bot added the product history We have kept 10 years of product revisions. This is useful to monitor edits & product improvements label Nov 29, 2022

sultanowski marked this pull request as draft November 29, 2022 18:56

github-actions bot added 💥 Merge Conflicts 💥 Merge Conflicts and removed 💥 Merge Conflicts 💥 Merge Conflicts labels Nov 30, 2022

sultanowski added 3 commits December 5, 2022 12:42

Merge branch 'main' into sultanowski-donation-banner

94cc22c

refator: added hide for producers platform class to banners.

04f9deb

refactor: done.

0a99d06

sultanowski marked this pull request as ready for review December 5, 2022 12:59

github-actions bot removed the 💥 Merge Conflicts 💥 Merge Conflicts label Dec 5, 2022

alexgarel approved these changes Dec 5, 2022

View reviewed changes

alexgarel merged commit d84b17c into openfoodfacts:main Dec 5, 2022

alexgarel mentioned this pull request Dec 5, 2022

chore(main): release 2.5.0 #7781

Merged

alexgarel mentioned this pull request Mar 13, 2023

<p> tags are display none in the CSS for Static pages #8188

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: donation banner tweaks #7703

fix: donation banner tweaks #7703

sultanowski commented Nov 15, 2022 •

edited by teolemon

Loading

alexgarel left a comment

alexgarel Nov 16, 2022

sultanowski Nov 16, 2022

alexgarel Nov 16, 2022

sultanowski Nov 16, 2022

alexgarel Nov 16, 2022

alexgarel Nov 16, 2022

alexgarel Nov 16, 2022

alexgarel Nov 16, 2022

stephanegigandet commented Nov 16, 2022

stephanegigandet commented Nov 16, 2022

stephanegigandet commented Nov 16, 2022

sultanowski commented Nov 16, 2022

alexgarel commented Nov 16, 2022

sultanowski commented Nov 16, 2022

alexgarel left a comment

sonarqubecloud bot commented Dec 5, 2022

sultanowski commented Dec 5, 2022

alexgarel commented Dec 5, 2022

fix: donation banner tweaks #7703

fix: donation banner tweaks #7703

Conversation

sultanowski commented Nov 15, 2022 • edited by teolemon Loading

What

Screenshot

Related issue(s) and discussion

alexgarel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephanegigandet commented Nov 16, 2022

stephanegigandet commented Nov 16, 2022

stephanegigandet commented Nov 16, 2022

sultanowski commented Nov 16, 2022

alexgarel commented Nov 16, 2022

sultanowski commented Nov 16, 2022

alexgarel left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Dec 5, 2022

sultanowski commented Dec 5, 2022

alexgarel commented Dec 5, 2022

sultanowski commented Nov 15, 2022 •

edited by teolemon

Loading