[harmony] Bug 1983391: Add a donation banner to the home page#156
[harmony] Bug 1983391: Add a donation banner to the home page#156justdave wants to merge 12 commits into
Conversation
|
No strong feeling if we keep the current list of options but it might be better to have less. For example,
Feel free to file a follow-up bug to explore this option if you consider it low priority for the next release ;) |
| border: 1px solid rgb(var(--accent-color-green-1)); | ||
| border-radius: 5px; | ||
| background: rgb(var(--accent-color-green-1)); |
There was a problem hiding this comment.
In dark mode, the contrast ratio is too low. We have two options:
- Use
--accent-color-lightgreen-1for the border and background color - Use
--accent-color-green-1withcolor: #000 !important
I, personally, find option 1 more aesthitically more pleasing.
There was a problem hiding this comment.
I added a follow-up using your version 1 suggestion.
| </td> | ||
| </tr> | ||
| <tr> | ||
| <td><label for="donate_banner_reminder_date">Date:</label></td> |
There was a problem hiding this comment.
When the date is non-specific, we should disable the datepicked to remove any confusion.
There was a problem hiding this comment.
Also dealt with this in a little follow-up
We don't need the settings exposed twice under 'Preferences' do we.. once without good descriptive names under 'Bugzilla.org' in the 'General preferences' and a second time in their own tab on the left 'Donate to Bugzilla' |
|
Nope, should only be on the donate tab. I'll fix that. |
|
OK, not just cosmetic.. there's some other leakage of those preferences going on...
Looking at the database, it looks like when saving settings from the 'General preferences' area, even if nothing changed.. we wipe the banner settings. |
|
Dismissing via the Donate tab doesn't actually dismiss
Looking at the database donate_banner_pref does get properly set to 'next_upgrade' but theres no donate_banner_last_version row |
|
I think the banner itself should match the width of the homepage.. it looks a bit odd having part of the page nicely centred with gutters and the banner not adhering to the same. |
|
The buggy picture is a bit stretched.. but you've already suggested that you'd like to improve that. |
|
I think I'd also display the banner on the 'Administration' page until it's dismissed. |
|
Inspecting the code a little.. it looks like we're using the same '#new_release' slot in the template as the security release warnings stuff.. Doesn't that mean that the donation banner hides the new-release / EOS notice I couldn't quit work out how to replicate that one for reals, but the code is fairly clear to follow.. I think the donation banner should be more distinct in the template. |
| if ($@) { | ||
| print "Skipping invalid group regexp for group $gid: $rexp\n"; | ||
| next; | ||
| } |
There was a problem hiding this comment.
This doesn't look related to the banner addition to me?
…te tab SaveDonate() set donate_banner_pref but never stamped donate_banner_last_version, so Bugzilla::Donation::get_banner() still saw last_version ne BUGZILLA_VERSION and kept showing the banner. Choosing 'Remind me after the next upgrade' in the Donate preferences tab therefore did not actually dismiss the banner, unlike the home page banner path (set_banner_preference) which stamps the version. Stamp BUGZILLA_VERSION when saving the donate preferences so both dismiss paths behave consistently.
…s tab The donation banner stores its per-user state as three user settings. donate_banner_last_version and donate_banner_reminder_date have no legal_values, so they rendered as empty dropdowns under a new 'Bugzilla.org' category in Preferences > General Preferences. Worse, because their only submitted value was the '<name>-isdefault' sentinel, saving that tab reset them to their defaults via reset_to_default(), which silently made a previously dismissed banner reappear. donate_banner_pref was also duplicated there alongside its dedicated Donate tab. These settings are managed from the Donate tab and the home page banner, so exclude all three from both the display (DoSettings) and the save loop (SaveSettings) of the generic Settings tab.
Bugzilla::User::Setting::Donation defined a custom validate_value(), but no setting declared subclass => 'Donation' in Bugzilla::Install::SETTINGS, so the class was never loaded. It is dead code on two counts: the subclass is unwired, and Bugzilla::User::Setting::set() writes directly to the database without calling validate_value() at all. The values it guarded are only ever set from trusted sources -- BUGZILLA_VERSION for the version, and validate_date()-checked input for the reminder date -- so no validation is lost by removing it.
index.html.tmpl rendered the donation banner and the new-release / end-of- support notice as mutually exclusive ([% IF donation %] ... [% ELSIF release %]), both sharing the #new_release element. Because the banner is shown to admins by default until dismissed, it suppressed the upgrade and end-of-support security notice from Bugzilla::Update::get_notifications(). Render the two as independent blocks so both can appear. The banner now uses its own .donation_banner container instead of reusing the #new_release id (which also avoids duplicate ids on the page); the CSS selectors are updated to the class and given the margin/padding they previously inherited from #new_release.
… banners The 'do not hide the new-release notice' change split the donation banner off the #new_release element, which dropped the shared box styling (margins, padding, and the small-print .notice styling) from the donation banner. Introduce a .home_banner base class carrying the shared box styling and .notice sizing, applied to both the #new_release and .donation_banner elements. Each banner keeps only its distinctive bits (red vs green border color, bold vs normal weight, the donation layout/shadow). The shared rule uses the border-width/-style longhands so it never resets each banner's own border-color regardless of stylesheet load order.
Bugzilla::Donation::get_banner() issued a session token on every home page render that showed the banner, writing a row to the tokens table each time and leaving it there until the (single-use) form was submitted or the row aged out. For a banner shown to every eligible user on every uncached home page view that is needless database churn. Switch to issue_hash_token()/check_hash_token(), which are stateless: the token is an HMAC of the user and a data tag, validated by recomputation with no database row and nothing to delete afterwards. Same CSRF protection, no churn.
buggie.png is a tall portrait image (78x215) but .donation_banner_image forces it into a 110x110 square, distorting it horizontally. Add object-fit: contain (centered) so Buggie keeps his aspect ratio and sits centered within the frame.
On the Donate preferences tab the date picker was always visible, even for the 'next upgrade' and 'never' choices where it does nothing, which is confusing. Hide the date row by default (unless the saved preference is specific_date) and toggle it from the reminder dropdown via a small CSP-nonced script.
The dim rgba(green, 0.6) border barely separated the banner from the dark page background. Use the brighter --accent-color-lightgreen-1 for the border (and a low-alpha lightgreen tint for the background) so the banner stands out while the light body text stays readable.
The banners rendered full width (~1250px at a 1280px viewport) while the welcome heading and tiles are a centered 640px column, so the banner looked oddly wide and disconnected. Move both banners inside #page-index and cap .home_banner at the content width (max-width: 640px, border-box) centered with auto side margins, so they line up with the tiles and gain proper gutters.
donate_banner_reminder_date defaults to 1970-01-01 as a 'show immediately' sentinel, which the Donate tab date picker rendered literally, forcing the user to navigate forward decades. Since a reminder is always a future date, fall back to today (computed in Bugzilla->local_timezone, matching get_banner and set_banner_preference) when the stored value is still the epoch sentinel.
Fixed in a follow-up above |
|
Whilst working through this I think I found bugs in how we're triggering update notifications and how bugzilla is versioned now.. the Bugzilla::Constants version (which take a semantic form) is basically ignored everywhere and instead we're using the Bugzilla->VERSION string (which is a date format string) instead... it's tied into all the CSS and JS static file endpoints too, so it's not a fix I think we should work on here.. I'll report a new bug for it |
3 participants
Details
Adds a donation banner to the front page, per the specification in Bug 1983391 comment 3
Light mode:
Dark mode:
The "call to action" message at the top is randomly picked from the following list:
I'm open to suggestion for messaging in there.
It could use a real graphic on the left, too. Buggie holding a wad of cash? @hellcp
The administrative params screen linked to in the bottom of that banner:
The end-user preferences screen:
Additional info
Test Plan