Lead generation modal for Spicule #25

barrymcgee · 2019-02-19T16:54:39Z

Done

Added lead generation form for Spicule

QA

Run feature branch
Run ./run serve
View http://0.0.0.0:8029/experts/spicule
Click 'Get in touch'

Notes

This form still needs to be connected to Marketo so remains blocked.

webteam-app · 2019-02-19T16:55:56Z

Starting demo at: https://jaas-ai-canonical-websites-pr-25.run.demo.haus/

clagom · 2019-03-11T14:01:15Z

Ok, we can proceed on with this. We discussed the issue with legal, and we can have a combined message with Canonical and Spicule with a thick box agreement with both private policies.

Tengu will stay as it is currently (no form), as the status of the partnership is unclear. Spicule can go on.

Sending the copy for the check box shortly later.
@barrymcgee

clagom · 2019-03-11T17:21:44Z

@barrymcgee, here's copy:
In submitting this form, I confirm that I have read and agree to Canonical’s Privacy Notice (LINK: https://www.ubuntu.com/legal/data-privacy/contact?_ga=2.67215766.606903877.1552321043-1465573742.1527584293) and Privacy Policy (LINK: https://www.ubuntu.com/legal/data-privacy?_ga=2.67215766.606903877.1552321043-1465573742.1527584293) and Spicule's Privacy Policy (LINK: https://spicule.co.uk/privacy-policy/) and acknowledge that I may be contacted by Spicule and/or Canonical.

anthonydillon · 2019-03-13T18:36:15Z

@barrymcgee can you add labels if you want reviews please

spencerbygraves · 2019-03-20T10:56:03Z

@barrymcgee does this need a design review?

webteam-app · 2019-03-20T14:43:22Z

Starting demo at: https://jaas-ai-canonical-websites-pr-25.run.demo.haus/

barrymcgee · 2019-03-20T14:52:03Z

@spencerbygraves Yes please:

Spicule experts page: https://jaas-ai-canonical-websites-pr-25.run.demo.haus/experts/spicule

Spicule supported bundle: https://jaas-ai-canonical-websites-pr-25.run.demo.haus/u/spiculecharms/anssr-data-engine

spencerbygraves · 2019-03-20T17:11:24Z

@barrymcgee it appears to work on the experts page but if I start to scroll this happens:

When viewing the bundle I have to scroll to see the button, so when I click this happens:

Also, I think the dialog being as wide as the page is unnecessary, something like half the width would be fine.

Thanks.

spencerbygraves · 2019-03-20T17:13:10Z

@barrymcgee I'm on holiday tomorrow and Friday, so please ask @clagom to take a look if another review is needed

clagom · 2019-03-26T10:11:26Z

Hi @barrymcgee, @spencerbygraves and I had a look at the demo, thank you.
On top of Spencer's comment up there, we can implement to close the modal by clicking on the dark background (this is a behaviour that we use across websites). I see the filled data is maintained if you close the modal and reopen it, so it shouldn't be a problem for that.
I think we don't need the title "Get in touch with Spicule", that would save us some space.

spencerbygraves

@barrymcgee it appears to work on the experts page but if I start to scroll this happens:

When viewing the bundle I have to scroll to see the button, so when I click this happens:

Also, could you update a couple of things please:

I think the dialog being as wide as the page is unnecessary, in the design it's much narrower.
Can you adjust the spacing above and below the hr please:
Can we group the two telephone numbers under the one icon please, see the design
And the privacy message should be 14px please

Thanks

spencerbygraves

Thanks @barrymcgee looking good.

Just two small things then both @clagom and I are happy.

Can the 14px privacy text have a line-height of 1.25rem please, as in Vanilla
Can you move the send button to the right please

Thanks 👍

spencerbygraves

@barrymcgee thanks for updating. Could you just add 'rem' to the line-height, 1.25; please, it's not displaying correctly without it.

Also I missed the padding left on the contact list, could you remove it please so everything aligns on the left:

Thanks

barrymcgee · 2019-03-27T14:20:05Z

@spencerbygraves Demo service playing up - here's a screenshot instead

spencerbygraves · 2019-03-27T14:37:03Z

@barrymcgee looks good thanks for updating 👍

spencerbygraves

👍

hatched · 2019-03-27T15:22:09Z

.circleci/config.yml

@@ -17,7 +16,7 @@ jobs:
          command: pip3 install flake8
      - run:
          name: Lint webapp with Flake8
-          command: flake8 webapp
+          command: flake8 --max-line-length 95 webapp


this will be overridden by #79

static/js/app/modal.js

hatched · 2019-03-27T15:28:08Z

templates/jaasai/experts/spicule.html

@@ -187,5 +150,9 @@ <h4>
    </div>
  </div>
 </div>
+{% include "shared/_spicule_lead-gen-form.html" %}
+<script>
+  window.app.modal('leadGenerationModal');


Because the lead gen form is the one that actually includes the JS onto the page I feel like the instantiation of that modal should also be in that template. Doing it this way leads to the possibility of one or the other being removed accidentally.

But then you hardwire the id of the form within the partial? You previously asked that I refactor this partial so there could be more than one modal per page if needed?

In this case the partial is the lead generation form. And you're instantiating it as a lead generation form. If you were to create another modal you'd likely have another partial with a different name.

hatched · 2019-03-27T15:28:27Z

templates/store/bundle-details.html

+{% endif %}
+
+<script>
+  window.app.modal('leadGenerationModal');


See above comment about this initialization.

tests/store/test_views.py

webapp/store/views.py

hatched

Thanks @barrymcgee just a small change then it's good to land.

hatched · 2019-03-27T19:30:06Z

static/js/app/modal.js

+@param { String } modalId
+The DOM id of the element you'd like to turn into a modal.
+*/
+window.app.modal = function(modalId) {


Now that this is being included as a template it should check to see if the variable is available before assigning into it. In this case it's not really a problem but just a good practice

if (!window.app.model) { ... }

barrymcgee added the Blocked ⛔ label Feb 19, 2019

barrymcgee changed the base branch from master to develop March 11, 2019 09:34

clagom removed the Blocked ⛔ label Mar 12, 2019

barrymcgee force-pushed the lead-gen-modal-forms branch from 748a006 to f0196fc Compare March 13, 2019 11:30

barrymcgee marked this pull request as ready for review March 13, 2019 11:30

barrymcgee requested a review from clagom March 13, 2019 11:32

barrymcgee added the Review: QA needed label Mar 14, 2019

barrymcgee changed the base branch from develop to master March 19, 2019 15:55

barrymcgee force-pushed the lead-gen-modal-forms branch from 673650e to dcadaf2 Compare March 19, 2019 16:16

barrymcgee added Review: Design needed Review: Code needed labels Mar 20, 2019

barrymcgee requested a review from spencerbygraves March 20, 2019 14:42

barrymcgee force-pushed the lead-gen-modal-forms branch from 19f37ba to 273ba4b Compare March 20, 2019 14:59

barrymcgee assigned spencerbygraves Mar 20, 2019

spencerbygraves added Review: Design -1 and removed Review: Design needed labels Mar 26, 2019

spencerbygraves suggested changes Mar 26, 2019

View reviewed changes

barrymcgee force-pushed the lead-gen-modal-forms branch from 273ba4b to 9473a99 Compare March 26, 2019 12:37

barrymcgee requested a review from spencerbygraves March 26, 2019 16:26

barrymcgee force-pushed the lead-gen-modal-forms branch from bc0d42a to 1411498 Compare March 26, 2019 16:28

spencerbygraves suggested changes Mar 26, 2019

View reviewed changes

spencerbygraves suggested changes Mar 27, 2019

View reviewed changes

barrymcgee added Review: Design needed and removed Review: Design -1 labels Mar 27, 2019

spencerbygraves added Review: Design +1 and removed Review: Design needed labels Mar 27, 2019

spencerbygraves previously approved these changes Mar 27, 2019

View reviewed changes

hatched suggested changes Mar 27, 2019

View reviewed changes

barrymcgee dismissed spencerbygraves’s stale review via bdfdbb4 March 27, 2019 17:26

barrymcgee force-pushed the lead-gen-modal-forms branch from 6594f73 to a4c9726 Compare March 27, 2019 18:44

Add lead gen form for Spicule

1103eb0

barrymcgee force-pushed the lead-gen-modal-forms branch from a4c9726 to 1103eb0 Compare March 27, 2019 18:48

barrymcgee added 2 commits March 27, 2019 18:49

Add doc block comment

a1240d6

Move function call into partial

9873e00

hatched previously approved these changes Mar 27, 2019

View reviewed changes

hatched added Review: Code +1 Review: QA +1 and removed Review: Code needed Review: QA needed labels Mar 27, 2019

Add conditional check for global namespace

0539dde

barrymcgee dismissed hatched’s stale review via 0539dde March 27, 2019 19:40

barrymcgee added 4 commits March 27, 2019 20:06

Point thanks to correct page

2377f8c

Update flake8 lint config

97b28f2

Run package script directly from .circleci config

92abf1c

Fix linter

fb0b43c

hatched approved these changes Mar 27, 2019

View reviewed changes

hatched merged commit ce0159a into canonical:master Mar 27, 2019

barrymcgee deleted the lead-gen-modal-forms branch April 4, 2019 11:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lead generation modal for Spicule #25

Lead generation modal for Spicule #25

barrymcgee commented Feb 19, 2019 •

edited

Loading

webteam-app commented Feb 19, 2019

clagom commented Mar 11, 2019

clagom commented Mar 11, 2019

anthonydillon commented Mar 13, 2019

spencerbygraves commented Mar 20, 2019

webteam-app commented Mar 20, 2019

barrymcgee commented Mar 20, 2019 •

edited

Loading

spencerbygraves commented Mar 20, 2019

spencerbygraves commented Mar 20, 2019

clagom commented Mar 26, 2019

spencerbygraves left a comment

spencerbygraves left a comment

spencerbygraves left a comment

barrymcgee commented Mar 27, 2019

spencerbygraves commented Mar 27, 2019

spencerbygraves left a comment

hatched Mar 27, 2019

hatched Mar 27, 2019

barrymcgee Mar 27, 2019

hatched Mar 27, 2019

hatched Mar 27, 2019

hatched left a comment

hatched Mar 27, 2019

Lead generation modal for Spicule #25

Lead generation modal for Spicule #25

Conversation

barrymcgee commented Feb 19, 2019 • edited Loading

Done

QA

Notes

webteam-app commented Feb 19, 2019

clagom commented Mar 11, 2019

clagom commented Mar 11, 2019

anthonydillon commented Mar 13, 2019

spencerbygraves commented Mar 20, 2019

webteam-app commented Mar 20, 2019

barrymcgee commented Mar 20, 2019 • edited Loading

spencerbygraves commented Mar 20, 2019

spencerbygraves commented Mar 20, 2019

clagom commented Mar 26, 2019

spencerbygraves left a comment

Choose a reason for hiding this comment

spencerbygraves left a comment

Choose a reason for hiding this comment

spencerbygraves left a comment

Choose a reason for hiding this comment

barrymcgee commented Mar 27, 2019

spencerbygraves commented Mar 27, 2019

spencerbygraves left a comment

Choose a reason for hiding this comment

hatched Mar 27, 2019

Choose a reason for hiding this comment

hatched Mar 27, 2019

Choose a reason for hiding this comment

barrymcgee Mar 27, 2019

Choose a reason for hiding this comment

hatched Mar 27, 2019

Choose a reason for hiding this comment

hatched Mar 27, 2019

Choose a reason for hiding this comment

hatched left a comment

Choose a reason for hiding this comment

hatched Mar 27, 2019

Choose a reason for hiding this comment

barrymcgee commented Feb 19, 2019 •

edited

Loading

barrymcgee commented Mar 20, 2019 •

edited

Loading