Adding News Letter Notice to Draw Attention #51
Conversation
…om/nsquared-team/draw-attention into implementing-news-letter-component
|
Hello @gnarza @NatalieMac @tylerdigital here is a quick demo to go over the implementation of the news letter metabox component. Thank you for your time. |
|
Hello @NatalieMac here is the latest demo update for the requested changes. Thank you for your time! |
| } | ||
| }); | ||
|
|
||
| document.addEventListener("keydown", function (event) { |
There was a problem hiding this comment.
I'm going to go ahead and pass this since I don't anticipate it causing issues for this case, but generally, you want to attach this event listener fro the escape key when the modal opens and then remove it from the body once the modal is closed.
There was a problem hiding this comment.
Thank you! I'll keep that in mind.
public/includes/da-newsletter.php
Outdated
| } | ||
|
|
||
| public function enqueue_meta_box_assets() { | ||
| wp_enqueue_style( 'custom-meta-box-styles', $this->plugin_directory . '/assets/css/custom-meta-box-styles.css', array(), DrawAttention::VERSION ); |
There was a problem hiding this comment.
@Majed-Habli we need to namespace scripts/styles since this will cause conflicts if any other plugin has a style custom-meta-box-styles, so this will need to become da-custom-meta-box-styles wherever it's registered and enqueued
public/includes/da-newsletter.php
Outdated
|
|
||
| public function enqueue_meta_box_assets() { | ||
| wp_enqueue_style( 'custom-meta-box-styles', $this->plugin_directory . '/assets/css/custom-meta-box-styles.css', array(), DrawAttention::VERSION ); | ||
| wp_enqueue_script( 'news-letter-js', $this->plugin_directory . 'assets/js/news-letter.js', array(), DrawAttention::VERSION ); |
There was a problem hiding this comment.
same here, da- prefix
| </div> | ||
| <div class='outer-content'> | ||
| <p>Subscribe now! Get 20% Coupon</p> | ||
| <button id='openModalButton' onclick='openModal()'> <span>" . __( 'SUBSCRIBE', 'draw-attention' ) . "</span> </button> |
There was a problem hiding this comment.
Let's change to Subscribe and apply text-transform: uppercase to achieve the all caps
public/includes/da-newsletter.php
Outdated
|
|
||
| global $wp_meta_boxes; | ||
|
|
||
| if ( 'da_image' == $post_type ) { |
There was a problem hiding this comment.
There was a problem hiding this comment.
Thank you @tylerdigital for the explanation! i'll start making it a habit to do that instead 👍.
…om/nsquared-team/draw-attention into implementing-news-letter-component
|
@tylerdigital Updated the code as requested. Thank you. |
There was a problem hiding this comment.
I have some small requests
- In the screen options, fix small typo "News Letter" to "Newsletter"
- for the fonts used in all the Newsletter components, let's switch to a sans-serif font to match the figma more similarly
- the email font-color when entering my email should be darker it's difficult to see what I've entered
- the confirmation screen shouldn't take me to a new page, if it does then we need to make it simple to get back to the DA editor page/WordPress
And, my submission didn't go through to ActiveCampaign - is it hooked up yet?
Tested here: https://da-pr-51.sandbox.ssa.rocks/wp-admin
|
@gnarza, I've applied the requested modifications. Can you please check if the active campaign allows us to customize the confirmation page over on their site ? |
There was a problem hiding this comment.
- In the screen options, fix small typo "News Letter" to "Newsletter" ✅
- for the fonts used in all the Newsletter components, let's switch to a sans-serif font to match the figma more similarly ❌ still using the same fonts as before on sandbox
- the email font-color when entering my email should be darker it's difficult to see what I've entered ❌ still pale grey when inputting the email
- the confirmation screen shouldn't take me to a new page, if it does then we need to make it simple to get back to the DA editor page/WordPress ✅ I have to adjust it in ActiveCampaign for now I have it going to the pricing page
- submission ✅ submission went through successfully for me now
https://www.loom.com/share/ff90dcdfddbb4408b65099a5ef3c5753?sid=8295b2d1-6b64-4f53-bc12-0664b41e3e90
Tested on same sandbox with new pr zip https://da-pr-51.sandbox.ssa.rocks/wp-admin
|
@gnarza i tired running it, everything looks good. I think it might have been a cache issue or that the sandbox somehow wasn't fully loaded? |
https://app.asana.com/0/1202993069895285/1206073108876520/f