Add newsletter form and basic button #112
Conversation
|
Is |
|
RTL styles are a little wonky https://screenshots.firefox.com/qShhAExCQZXpa4gN/localhost |
|
|
| text-align: center; | ||
| } | ||
|
|
||
| .mzp-c-button { |
There was a problem hiding this comment.
Would this be worth having as a button style atom itself, as opposed to something specific to the newsletter form?
There was a problem hiding this comment.
Quite possibly, but I think Vince/Jennifer are still working on some button styles so I don't know if this will stick.
| <p> | ||
| <label for="privacy" class="mzp-u-inline"> | ||
| <input type="checkbox" id="privacy" name="privacy" required aria-required="true"> | ||
| I’m okay with Mozilla handling my info as explained in <a href="#">this Privacy Notice</a> |
There was a problem hiding this comment.
Should we link to the actual privacy policy in the example?
| </fieldset> | ||
| </form> | ||
|
|
||
| <div id="newsletter-thanks" class="mzp-c-newsletter-thanks"> |
There was a problem hiding this comment.
Is there any way to see these styles in the example without the form being functional?
There was a problem hiding this comment.
I've set up the demo page to show different states of the form (normal, errors, success).
| var formControls = document.querySelectorAll('.mzp-c-newsletter-form input, .mzp-c-newsletter-form select'); | ||
|
|
||
| function emailFormShowDetails() { | ||
| if (window.getComputedStyle(formDetails).display === 'none') { |
There was a problem hiding this comment.
Rather than use thesegetComputedStyle calls, it may be slightly more performant to use a boolean variable to determine if the form has expanded? e.g.
if (!formExpanded) {
formDetails.style.display = 'block';
formExpanded = true;
}
craigcook
force-pushed
the
newsletter-form
branch
from
9a6db15
to
673c77d
Compare
|
|
||
| for (var i = 0; i < formControls.length; i++) { | ||
| formControls[i].addEventListener('focus', function() { | ||
| emailFormShowDetails(); |
There was a problem hiding this comment.
Do we need to add if (!formExpanded) { } here?
There was a problem hiding this comment.
The emailFormShowDetails function already checks for it, but we need to check if the form is expanded on the submit button just for the sake of e.preventDefault. I'm sure there's a more elegant way to do this... I'm not good at JavaScript.
| var formControls = document.querySelectorAll('.mzp-c-newsletter-form input, .mzp-c-newsletter-form select'); | ||
| var formExpanded; | ||
|
|
||
| if (window.getComputedStyle(formDetails).display === 'none') { |
There was a problem hiding this comment.
If this is condition falsy then the value for formExpanded will be null instead of true. Maybe something like this could work?
var formExpanded = window.getComputedStyle(formDetails).display === 'none' ? false : true;
src/pages/demos/newsletter.hbs
Outdated
| <p>With errors.</p> | ||
|
|
||
| <div class="mzp-qa-newsletter-errors"> | ||
| <aside class="mzp-c-newsletter"> |
craigcook
force-pushed
the
newsletter-form
branch
from
673c77d
to
22aff88
Compare
craigcook
force-pushed
the
newsletter-form
branch
from
22aff88
to
72e2922
Compare
This adds a newsletter subscription form molecule as it appears on the home page. We'll probably come up with a handful of variants in due time. It also adds a basic button style that we probably need to expand upon, but the form needed a button so we might as well make it an atom.
Marking this WIP initially since there should be some discussion around the button style as well as how to handle the # form functionality.
Right now this uses a slight reformulation of the basket-client.js from https://github.com/mozilla/basket-example/ but upon further thought I think we should keep the Protocol component minimal and refer people to that external repo for the subscription functionality. I'm working on that next so updates to this PR are forthcoming.
https://demo2--mozilla-protocol.netlify.com/patterns/organisms/newsletter.html
https://demo2--mozilla-protocol.netlify.com/demos/newsletter.html