Initial Development #1

localjo · 2014-09-04T23:31:00Z

No description provided.

… UMD definition - Make the README less opinionated about how to use the module - Add to README: DOM element, jQuery element or selector stings as ways to select the element. - Add information about default template to README - Add details about configuration options to README - Add UMD definition to module Branch: MmLRZ2E2-development Branch: MmLRZ2E2-development Branch: MmLRZ2E2-development

localjo · 2014-09-04T23:37:50Z

Do we need to be concerned at all about what happens when a visitor has JavaScript disabled, or are we leaving that concern up to the developer who use this?

Branch: MmLRZ2E2-development

pushred · 2014-09-08T17:23:24Z

package.json

+  "name": "subscribe-email",
+  "version": "0.0.0",
+  "private": true,
+  "main": "subscribe-email.js",


This should be src/subscribe-email.js right?

oops, yep. I moved that and forgot to update package.json. I'll update it with the next commit.

Add support for Universe API Branch: MmLRZ2E2-development Branch: MmLRZ2E2-development

localjo · 2014-09-09T16:34:54Z

Concerning email validation, modern browsers validate <input type="email">, and Universe and the other platforms we support should be validating the email addresses server-side as well (not actually sure if they do, but I'd be surprised if they didn't). We could rely on those layers of validation instead of validating the email addresses in JavaScript. @pushred what are your thoughts on that?

Add an event container to template HTML and add 'subscribe-email-message' event Branch: MmLRZ2E2-development

pushred · 2014-09-09T18:51:59Z

Yeah I'm fine with relying on the server's own validation / messaging. It's a request, but no biggie.

setting template to false will now override the template with the markup from the page. Branch: no-template Branch: MmLRZ2E2-development

\o/ Branch: MmLRZ2E2-development

Branch: MmLRZ2E2-development

I think this works 🙈 Branch: MmLRZ2E2-development

localjo · 2014-09-12T17:40:13Z

I refactored this as a node-flavored module and then used Browserify's standalone option to build it as a UMD module. @pushred if you want to give this a whirl, you can just add "subscribe-email": "git://github.com/blocks/subscribe-email#4bc3e0" to an existing project's package.json. You can then require it using Browserify (or however you want), and initialize it on a form element with;

SubscribeEmail.init({
    element: '#subscribe-form',
    template: true,
    service: 'universe',
    key: 'd54e8487-e44e-4c6f-bdd7-6ab9c2eae1e9'
});

I think this resolves #2

pushred · 2014-09-12T18:23:37Z

Hmm sadly no.. I'm still seeing all the errors I listed there when Browserify is bundling my project. Confirmed that node_modules/subscribe_email/package.json has "main": "build/subscribe-email.js". Also tried the same entry point with Beefy and it stops with the same issue:

Error: Cannot find module './templates/BEM-with-messaging.hbs' from '/project/node_modules/subscribe-email/build'

so browserified module can be browserified without errors Branch: MmLRZ2E2-development

Branch: MmLRZ2E2-development

include a this instanceof SubscribeEmail check to let people consume the module with new SubscribeEmail or SubscribeEmail() Branch: MmLRZ2E2-development

configuring the hbsfy transform in package.json was causing problems when the module was included as a dependency in other projects. Branch: MmLRZ2E2-development

Add events API, clean up documentation and implementation details Branch: MmLRZ2E2-development

pushred · 2014-09-17T01:18:42Z

gulp/browserify.js

+var derequire    = require('gulp-derequire');
+
+var hbsfy = require('hbsfy').configure({
+  extensions: ['hbs']


Isn't this only required for custom extensions, i.e. .handlebars? .hbs is the usual convention that hbsfy follows.

update readme, remove unnecessary extensions parameter from browserify, move default template to flatten directory structure Branch: MmLRZ2E2-development

Add a basic test with Tape and Testling Branch: MmLRZ2E2-development

joanniclaborde · 2014-10-28T01:05:35Z

Yeah I had trouble running the tests in IE9 too, and I couldn't find a solution. The tests would run once in a while, but most of the time nothing would happen. I gave up on it for now...

tests won't run in ie9 without a doctype that triggers standards mode Branch: MmLRZ2E2-development

add event tests to make sure subscriptionMessage, subscriptionError, and subscriptionSuccess are fired when they should be Branch: MmLRZ2E2-development

localjo · 2014-10-29T20:30:59Z

247 tests passing! \o/ Looks like all tests are passing in all browsers.

update readme, and remove my personal mailchimp url from tests since it's no longer needed Branch: MmLRZ2E2-development

rather than inserting the template inside of the placeholder, the placeholder is now completely replaced by the template, which means the template contains the entire markup of the subscription form including the form tag itself. Branch: MmLRZ2E2-development

Branch: MmLRZ2E2-development

joanniclaborde · 2014-11-11T21:49:09Z

src/subscribe-email.js

+
+  if (typeof options.template === 'function') {
+    instance.template = options.template;
+    delete options.template;


Why delete the option? Also, you could just test for if (options.template) {.

I think originally I was deleting it because I thought it could cause issues to pass options.template into instance.template if they were the same thing. (Later, I do instance.template(options);) But I guess it's not necessary to delete it. I want to check for typeof options.template because I want to make sure that it's a valid compiled Handlebars template.

namespace module name with "blocks-", update alerter dependency to npm repo instead of github, remove global option from hbsfy transform, remove unnecessary code, update Events test to just test if object has emit method since specific events are tested elsewhere, update custom template test to check that the custom template is used and not just the default, change test API keys to be fake keys, remove duplicate prependMessagesTo docs Branch: MmLRZ2E2-development

namespace module name with "blocks-", update alerter dependency to npm repo instead of github, remove global option from hbsfy transform, remove unnecessary code, update Events test to just test if object has emit method since specific events are tested elsewhere, update custom template test to check that the custom template is used and not just the default, change test API keys to be fake keys, update README by adding better examples and remove duplicate prependMessagesTo docs Branch: MmLRZ2E2-development

…cribe-email into MmLRZ2E2-development

pushred · 2014-11-12T18:45:57Z

src/subscribe-email.js

+
+  switch (options.service) {
+    case 'universe':
+      options.formAction = options.formAction || 'http://services.sparkart.net/api/v1/contacts';


This should be https

Branch: MmLRZ2E2-development

pushred · 2014-11-12T19:21:44Z

test/selenium-driver.js

+  {browser: 'Chrome'},
+  {browser: 'Chrome', browser_version: '35.0'},
+  {browser: 'Safari'},
+  {browser: 'Safari', browser_version: '6.1'},


Safari's up to 8 now, Firefox's at 32, and Chrome's on 38. This'll be a pain to keep in sync across the modules, how about adding a super small module hosted on a repo in this org where we can define these centrally? Also are there any feeds we should be following from BrowserStack for when they add new versions? Would be great to automate that someday, but for now someone can just kick off tests locally whenever a new version is added.

Yeah. The nice part is that the latest version will always be tested by default. It's the previous version that will be difficult to keep in sync. Unfortunately, BrowserStack doesn't have a way of automatically selecting the previous version of a browser. I already contacted their support about this. I'm also not aware of anything except this page that has the latest versions published. I did, however, find browserstack-runner which has a previous version option, and could be useful either to use in our projects or to learn from. I'll look into how it works.

I gave browserstack-runner a shot, and it's pretty promising, but there are still some issues that will prevent us from switching right away: browserstack/browserstack-runner#94

I've opened an issue here to address this.

pushred · 2014-11-12T22:11:11Z

Trying the latest locally I'm running into that issue where the Handlebars precompiled template source is rendered into the page where the alert's supposed to go. Checking blocks-subscribe/node_modules/blocks-alert/package.json I'm seeing 0.0.2. Is another release needed to fix that issue?

localjo · 2014-11-13T14:56:27Z

@pushred I found out that that issue is caused by the template being browserified twice. Whether that's because it's in a gulp task and package.json, or applied to the dependent module and then again, globally, from the parent module. So that should help narrow down why you're seeing that, but I'm not able to replicate that finding. Also, which package are you seeing @0.0.2?

pushred · 2014-11-13T16:55:55Z

Oops I meant 1.0.2, gotta get used to this insane 1.x versioning! I just removed the global transform I had in my parent project, but now Browserify gets stuck:

Running "browserify:scripts" (browserify) task
>> Error: Parsing file /Users/eric/dev/sites/funimpact.org/node_modules/blocks-subscribe/src/subscribe-form.hbs: Line 5: Expected corresponding XJS closing tag for input

Did you forget to push up the transform in this module's config?

pushred · 2014-11-13T17:36:12Z

Yeah it's all good once I add that.

to avoid double transforms that cause the source code text to be rendered into the page Branch: MmLRZ2E2-development

add more descriptive name and better keywords Branch: MmLRZ2E2-development

Initial Development

[#MmLRZ2E2] Add templates and render into demo, add gulp

327f21b

Branch: MmLRZ2E2-development

pushred reviewed Sep 8, 2014
View reviewed changes

[#MmLRZ2E2] Add Universe Support

79c0cfb

Add support for Universe API Branch: MmLRZ2E2-development Branch: MmLRZ2E2-development

localjo force-pushed the MmLRZ2E2-development branch from 53cdbea to 79c0cfb Compare September 9, 2014 14:38

[#MmLRZ2E2] Add Messaging and Events

5376e6f

Add an event container to template HTML and add 'subscribe-email-message' event Branch: MmLRZ2E2-development

[#MmLRZ2E2] Add option to override the template

e8b4c40

setting template to false will now override the template with the markup from the page. Branch: no-template Branch: MmLRZ2E2-development

localjo force-pushed the MmLRZ2E2-development branch from ca5c493 to e8b4c40 Compare September 10, 2014 18:22

localjo added 3 commits September 10, 2014 18:03

[#MmLRZ2E2] Add SendGrid Support

4760fdf

\o/ Branch: MmLRZ2E2-development

[#MmLRZ2E2] refactor as node module

84e90a5

Branch: MmLRZ2E2-development

[#MmLRZ2E2] package as UMD

4bc3e0f

I think this works 🙈 Branch: MmLRZ2E2-development

localjo added 2 commits September 12, 2014 16:47

[#MmLRZ2E2] add derequire

6cb8dfe

so browserified module can be browserified without errors Branch: MmLRZ2E2-development

[#MmLRZ2E2] move handlebars to dependencies instead of dev

23d151e

Branch: MmLRZ2E2-development

localjo mentioned this pull request Sep 12, 2014

Can’t require module with Browserify #2

Closed

localjo added 4 commits September 15, 2014 11:10

[#MmLRZ2E2] add default options and make template BEM

8cc6ab6

Branch: MmLRZ2E2-development

[#MmLRZ2E2] include instanceof check

897ae33

include a this instanceof SubscribeEmail check to let people consume the module with new SubscribeEmail or SubscribeEmail() Branch: MmLRZ2E2-development

[#MmLRZ2E2] move hbsfy transform to gulp

1b060b8

configuring the hbsfy transform in package.json was causing problems when the module was included as a dependency in other projects. Branch: MmLRZ2E2-development

[#MmLRZ2E2] Add events API, clean up docs

d95e7bc

Add events API, clean up documentation and implementation details Branch: MmLRZ2E2-development

pushred reviewed Sep 17, 2014
View reviewed changes

localjo added 2 commits September 17, 2014 12:02

[#MmLRZ2E2] update readme and simplify some things

419128f

update readme, remove unnecessary extensions parameter from browserify, move default template to flatten directory structure Branch: MmLRZ2E2-development

[#MmLRZ2E2] Add tests

03f114e

Add a basic test with Tape and Testling Branch: MmLRZ2E2-development

localjo added 2 commits October 28, 2014 17:13

[#MmLRZ2E2] add doctype to mocha test page for ie9

3593ab1

tests won't run in ie9 without a doctype that triggers standards mode Branch: MmLRZ2E2-development

[#MmLRZ2E2] add event tests

d969836

add event tests to make sure subscriptionMessage, subscriptionError, and subscriptionSuccess are fired when they should be Branch: MmLRZ2E2-development

localjo added 3 commits October 29, 2014 16:33

[#MmLRZ2E2] update readme

b1bc875

update readme, and remove my personal mailchimp url from tests since it's no longer needed Branch: MmLRZ2E2-development

[#MmLRZ2E2] add friendlier message for mocha test errors

eafad14

Branch: MmLRZ2E2-development

joanniclaborde reviewed Nov 11, 2014
View reviewed changes

localjo added 3 commits November 12, 2014 13:11

Merge branch 'MmLRZ2E2-development' of https://github.com/blocks/subs…

8fb5dce

…cribe-email into MmLRZ2E2-development

pushred reviewed Nov 12, 2014
View reviewed changes

[#MmLRZ2E2] change api urls to https

86a23a8

Branch: MmLRZ2E2-development

pushred reviewed Nov 12, 2014
View reviewed changes

localjo mentioned this pull request Nov 12, 2014

mocha: tests run, but session times out browserstack/browserstack-runner#94

Closed

localjo added 2 commits November 13, 2014 14:11

[#MmLRZ2E2] move hbsfy to package.json instead of gulp task

4eaeffc

to avoid double transforms that cause the source code text to be rendered into the page Branch: MmLRZ2E2-development

[#MmLRZ2E2] update metadata

d7090d9

add more descriptive name and better keywords Branch: MmLRZ2E2-development

localjo added a commit that referenced this pull request Nov 13, 2014

Merge pull request #1 from blocks/MmLRZ2E2-development

8b494e1

Initial Development

localjo merged commit 8b494e1 into master Nov 13, 2014

localjo deleted the MmLRZ2E2-development branch February 16, 2015 15:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial Development #1

Initial Development #1

localjo commented Sep 4, 2014

localjo commented Sep 4, 2014

pushred Sep 8, 2014

localjo Sep 8, 2014

localjo commented Sep 9, 2014

pushred commented Sep 9, 2014

localjo commented Sep 12, 2014

pushred commented Sep 12, 2014

pushred Sep 17, 2014

localjo Sep 17, 2014

joanniclaborde commented Oct 28, 2014

localjo commented Oct 29, 2014

joanniclaborde Nov 11, 2014

localjo Nov 12, 2014

pushred Nov 12, 2014

pushred Nov 12, 2014

localjo Nov 12, 2014

localjo Nov 13, 2014

pushred commented Nov 12, 2014

localjo commented Nov 13, 2014

pushred commented Nov 13, 2014

pushred commented Nov 13, 2014

Initial Development #1

Initial Development #1

Conversation

localjo commented Sep 4, 2014

localjo commented Sep 4, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

localjo commented Sep 9, 2014

pushred commented Sep 9, 2014

localjo commented Sep 12, 2014

pushred commented Sep 12, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joanniclaborde commented Oct 28, 2014

localjo commented Oct 29, 2014

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

pushred commented Nov 12, 2014

localjo commented Nov 13, 2014

pushred commented Nov 13, 2014

pushred commented Nov 13, 2014