Skip to content

Conversation

@empiricompany
Copy link
Contributor

@empiricompany empiricompany commented Jul 13, 2024

Description (*)

Add a new widget with newsletter subscription form.
It is useful for me to be able to insert the newsletter subscription form into CMS pages or Static Blocks.
I have always manually inserted this piece of code:
{{block type="newsletter/subscribe" template="newsletter/subscribe.phtml"}}

With the widget become:
{{widget type="newsletter/widget_subscribe" template="newsletter/subscribe.phtml"}}

But is the same, if you look the new block widget subscribe only extend block subscribe

class Mage_Newsletter_Block_Widget_Subscribe extends Mage_Newsletter_Block_Subscribe implements Mage_Widget_Block_Interface

Manual testing scenarios (*)

  1. Insert Newsletter Subscribe Form Widget
  2. Test with cache if form_key correctly change

demo

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added the Component: Newsletter Relates to Mage_Newsletter label Jul 13, 2024
@github-actions github-actions bot added the translations Relates to app/locale label Jul 13, 2024
@addison74
Copy link
Contributor

addison74 commented Jul 13, 2024

This is a good idea. Maybe we can extend it into another PR to the contact form too. But this would a little bit complex.

@empiricompany empiricompany requested a review from pquerner July 14, 2024 09:55
@fballiano fballiano changed the title Add a new widget with Newsletter Subscription Form New feature: Added "Newsletter Subscription Form" widget Jul 16, 2024
@fballiano
Copy link
Contributor

I would prefer not having the constructor only calling parent constructor, but I'll merge it anyway in a couple of days if we don't get more feedback

@empiricompany
Copy link
Contributor Author

I would prefer not having the constructor only calling parent constructor, but I'll merge it anyway in a couple of days if we don't get more feedback

I can remove it it's not a problem

addison74
addison74 previously approved these changes Jul 21, 2024
Copy link
Contributor

@addison74 addison74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the PR. It adds to the list the newsletter widget that I used it on a CMS page. In the Frontend the form is displayed as expected and I successfully subscribed.

@addison74
Copy link
Contributor

@elidrissidev, @kiatng - please stamp this PR with your green color.

elidrissidev
elidrissidev previously approved these changes Jul 22, 2024
@empiricompany empiricompany dismissed stale reviews from elidrissidev and addison74 via 1f3d6b4 July 30, 2024 06:53
@elidrissidev elidrissidev merged commit 46bf3ba into OpenMage:main Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Newsletter Relates to Mage_Newsletter translations Relates to app/locale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants