[fix bug 1254309] - Update Fx friends signup #3958

jpetto · 2016-03-10T00:02:01Z

Description

Instead of linking off to friends.mozilla.org, we instead want to present a newsletter subscribe form on mozilla.org/contribute/friends/.

Bugzilla

bug 1254309

Testing

Click the "Sign Me Up" button in the header area to reveal the sign up form
Complete the sign up form and ensure thank you message is displayed
Check no-JS and RTL

Checklist

Requires l10n changes.
Related functional & integration tests passing.

Waiting on new newsletter to be created (bug 1254685) and for GA to be verified (bug 1254368).

flodolo · 2016-03-10T05:47:28Z

bedrock/mozorg/templates/mozorg/contribute/friends.html

-  <h1>{{_('Join Firefox Friends today!')}}</h1>
-  <p>{{_('Step to the frontline to help us tell the world how awesome Firefox is and about all the important work Mozilla does every day.')}}</p>
-  <a href="https://friends.mozilla.org" id="ff-join" class="ff-join-now" rel="external">{{_('Join now')}}</a>
+  <h1>{{ _('Sign up. Read up. Pass it on.') }}</h1>


I don't think copy can get more American than this 👎

jpetto · 2016-03-10T17:03:11Z

bedrock/mozorg/templates/mozorg/contribute/friends.html

+{% if l10n_has_tag('contribute_friends_signup_update') %}
+  {% include 'mozorg/contribute/friends-new.html' %}
+{% else %}
+  {% include 'mozorg/contribute/friends-old.html' %}


@flodolo - Since so many strings have changed, will something like this work? I'd rather not have a single template littered with l10n_has_tag conditionals.

I think it makes sense, I'd just need to test it locally to verify that everything works as expected when it comes to template activations.

Ok, great. When you verify it works from an L10n standpoint, can you report back? I'll open it up for code review after.

Thanks!

jpetto · 2016-03-10T19:32:03Z

Note - this page is intended to be en-US only for the time being.

jpetto · 2016-03-10T19:36:15Z

We are still blocked on a new newsletter being created (TODO marked in code) and GA review from Gareth (some potential GA calls currently commented out), but otherwise code is now ready for review.

@alexgibson r?

jpetto · 2016-03-10T22:02:35Z

Tested in Fx, Chrome, Safari, and IE 6 - Edge. Aside from varying levels of (expected) UI roughness in IE 6 -10, signup form is functional.

alexgibson · 2016-03-11T13:28:24Z

bedrock/mozorg/views.py

+
+
+def contribute_friends(request):
+    # TODO: update newsletter ID - https://bugzilla.mozilla.org/show_bug.cgi?id=1254685


Just noting the #TODO here for clarity sake

alexgibson · 2016-03-11T13:34:16Z

Signup button is cropped on small screens:

alexgibson · 2016-03-11T13:42:03Z

media/js/mozorg/contribute/friends.js

+                top: $ffSignupArea.offset().top - 60
+            });
+
+            $('#id_email').focus();


Curious, the focus() here stops Firefox performing the native smooth scroll?

Hm, this is odd. It only seems to prevent scrolling in Dev Edition and Nightly. Release and Beta work as expected. Simply calling focus() before smoothScroll seems to fix things, but I wonder if I should file a bug...

alexgibson · 2016-03-11T14:07:16Z

Also not a blocker for this PR - I'm not clear on whether or not Firefox Friends replaces Student Ambassadors? We still have these pages:

https://www.mozilla.org/en-US/contribute/studentambassadors/
https://www.mozilla.org/en-US/contribute/studentambassadors/join/

As far as I see, the student ambassadors page is still linked from the manifesto:

https://www.mozilla.org/en-US/about/manifesto/

Perhaps they are two entirely separate things still, in which case ignore me completely 😄

[edit] - Ignore this - I totally got my wires crossed. Firefox friends was a replacement for Affiliates

alexgibson · 2016-03-11T14:19:09Z

media/css/mozorg/contribute/friends.less

+        }
+
+        .how-wrapper {
+            .flexbox();


Assuming we're ok with this part of the layout looking broken in IE? (content is still readable and accessible, just flagging things here).

This actually doesn't look too bad in IE. On the versions that actually load the CSS (8 and up), only 10 looks a little bad. 8 and 9 just stack, and 11 renders correctly. 10 renders the items side by side, just all left justified.

I'm okay with the above, but I'm pretty much always okay with older browsers not being perfect. 😁

alexgibson · 2016-03-11T14:24:30Z

Looks good, just a few things to tidy up and an open question about student ambassadors, which might well be void anyway 👍

I'm assuming we're ok with parts of the layout / signup form not looking as expected in browsers that don't support flexbox for this page (fine with me if so 😉)

jpetto · 2016-03-30T16:46:24Z

@alexgibson - While we're still waiting on confirmation/testing from the project owner/basket side, the coding is complete. If you could review my latest commits, I'll squash so we can be ready to merge when given the green light. No rush though.

alexgibson · 2016-03-31T08:07:26Z

tests/pages/contribute/friends.py

+
+    _url = '{base_url}/{locale}/contribute/friends'
+
+    _privacy_policy_checkbox_locator = (By.ID, 'id_privacy')


Is this locator still needed?

Ah, nope. Will remove.

alexgibson · 2016-03-31T08:11:49Z

media/css/mozorg/contribute/friends.less

-            vertical-align: top;
+
+        h3 {
+            .font-size(1.2rem);


Nit: use px for consistency with rest of the file? (here and below)

If this is becoming a pain to manage, feel free to ignore it btw. But I'd like us to start using one convention consistently. Personally, I think 14px is easier to remember than something like 0.875rem (the end result is rem either way). But I would be happy to defer to other opinions.

These rem values are carryover from the existing/current file. Agree px is better though, so will update.

alexgibson · 2016-03-31T08:39:21Z

Could of small nits but otherwise looks good 👍

It would be nice to have some more test coverage for the form (e.g. test default expected default values, test form submission success/failure), but we can always follow up with this later if need be, so not a blocker.

alexgibson · 2016-03-31T16:26:53Z

r to squash and merge when ready 🍟

jpetto · 2016-03-31T16:45:02Z

Basket craziness has been resolved. Last thing before merge is for email template to be updated. Bug 1254685 will be updated to give the green light.

flodolo · 2016-03-31T16:49:19Z

Just to confirm: I need to take down the existing localizations, right?

jpetto · 2016-03-31T16:50:35Z

Yes, please take down existing localizations. The page is purposefully en-US only for now.

flodolo · 2016-03-31T16:58:30Z

Pages disabled in prod, will remove the files later in my periodic cleanups
mozilla-l10n/bedrock-l10n@7261c32

jpetto added WIP L10N labels Mar 10, 2016

flodolo reviewed Mar 10, 2016
View reviewed changes

jpetto force-pushed the bug-1254309-firefox-friends-signup-updates branch from e57f958 to 452d05c Compare March 10, 2016 15:18

jpetto reviewed Mar 10, 2016
View reviewed changes

jpetto force-pushed the bug-1254309-firefox-friends-signup-updates branch from 37fd43f to d2f3006 Compare March 10, 2016 19:27

jpetto added Do Not Merge ⚠️ and removed WIP labels Mar 10, 2016

alexgibson reviewed Mar 11, 2016
View reviewed changes

jpetto force-pushed the bug-1254309-firefox-friends-signup-updates branch from 33ded40 to 70fcfc3 Compare March 29, 2016 16:36

alexgibson reviewed Mar 31, 2016
View reviewed changes

[fix bug 1254309] - Update Fx friends signup

4cff437

jpetto force-pushed the bug-1254309-firefox-friends-signup-updates branch from 186f4c1 to 4cff437 Compare March 31, 2016 16:28

jpetto removed the L10N label Mar 31, 2016

jpetto removed the Do Not Merge ⚠️ label Apr 1, 2016

alexgibson merged commit 86b70ab into mozilla:master Apr 1, 2016

jpetto deleted the bug-1254309-firefox-friends-signup-updates branch July 27, 2016 21:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fix bug 1254309] - Update Fx friends signup #3958

[fix bug 1254309] - Update Fx friends signup #3958

jpetto commented Mar 10, 2016

flodolo Mar 10, 2016

jpetto Mar 10, 2016

flodolo Mar 10, 2016

jpetto Mar 10, 2016

jpetto commented Mar 10, 2016

jpetto commented Mar 10, 2016

jpetto commented Mar 10, 2016

alexgibson Mar 11, 2016

alexgibson commented Mar 11, 2016

alexgibson Mar 11, 2016

jpetto Mar 11, 2016

alexgibson commented Mar 11, 2016

alexgibson Mar 11, 2016

jpetto Mar 11, 2016

alexgibson commented Mar 11, 2016

jpetto commented Mar 30, 2016

alexgibson Mar 31, 2016

jpetto Mar 31, 2016

alexgibson Mar 31, 2016

alexgibson Mar 31, 2016

jpetto Mar 31, 2016

alexgibson commented Mar 31, 2016

alexgibson commented Mar 31, 2016

jpetto commented Mar 31, 2016

flodolo commented Mar 31, 2016

jpetto commented Mar 31, 2016

flodolo commented Mar 31, 2016



		def contribute_friends(request):
		# TODO: update newsletter ID - https://bugzilla.mozilla.org/show_bug.cgi?id=1254685


		_url = '{base_url}/{locale}/contribute/friends'

		_privacy_policy_checkbox_locator = (By.ID, 'id_privacy')

[fix bug 1254309] - Update Fx friends signup #3958

[fix bug 1254309] - Update Fx friends signup #3958

Conversation

jpetto commented Mar 10, 2016

Description

Bugzilla

Testing

Checklist

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

jpetto commented Mar 10, 2016

jpetto commented Mar 10, 2016

jpetto commented Mar 10, 2016

Choose a reason for hiding this comment

alexgibson commented Mar 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexgibson commented Mar 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexgibson commented Mar 11, 2016

jpetto commented Mar 30, 2016

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

alexgibson commented Mar 31, 2016

alexgibson commented Mar 31, 2016

jpetto commented Mar 31, 2016

flodolo commented Mar 31, 2016

jpetto commented Mar 31, 2016

flodolo commented Mar 31, 2016