Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extends email token documentation in emails.rst #194

Open
wants to merge 7 commits into
base: 5.x
Choose a base branch
from

Conversation

putzwasser
Copy link

No description provided.

Copy link
Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

Hi @putzwasser, quite a few changes here - I'm not sure why the linter isn't picking them up and I'm having some problems with my localhost but hopefully the feedback is sufficient!

You can replace a custom token using the events ``$event->addToken($token, $contentToReplaceToken)``.


Basic Token Replacement
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Basic Token Replacement
Basic token replacement

Please don't use camel case - we use sentence case in headings.

Copy link
Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

Hey @putzwasser sorry it's taken us so long to get this over the line.

Do you think we can get the headings fixed in this file? I also fixed a camel case heading as well. Happy to make the commits myself if you haven't got the time to fix it.

.. Note:: Extending generally works by hooking into events using event listeners or subscribers. Read more about :doc:`listeners and subscribers</plugins/event_listeners>`.

Email tokens
------------
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
------------
************

Please use the correct headings:

H1 ###
H2 ***
H3 ===
H4 ---
H5 ~~~

This is a H2 so it should be *** not ---

- ``$event->addToken($uniqueId, $htmlContent)`` allows you to show the Email token in the email builder, so that users can easily add the token to their emails.
- ``$event->getContent()`` and ``$event->setContent()`` are used for replacing the Email token with actual Dynamic Content once the Email gets send or viewed in the browser.
Registering custom tokens in builders
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Copy link
Member

Choose a reason for hiding this comment

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

See earlier comment about headings. This should be a H3 if you want it to be nested under the H2 above.

You can either hard code your tokens textual description in ``$htmlContent`` or use a translatable string.

Rendering custom tokens
~~~~~~~~~~~~~~~~~~~~~~~
Copy link
Member

Choose a reason for hiding this comment

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

See above about headings.



Basic Token Replacement
^^^^^^^
Copy link
Member

Choose a reason for hiding this comment

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

Heading also needs fixing here please.



Email A/B testing
-----------------
Copy link
Member

Choose a reason for hiding this comment

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

Heading nesting needs fixing here too, I think perhaps it should be an H2 heading?


While Mautic supports :xref:`A/B testing` out of the box, you might have more complex needs to determine A/B test winner criteria.

- ``$event->addAbTestWinnerCriteria()`` allows you to do exactly that. Using your custom logic, you can decide the winner of such criteria. An example is shown below.
- ``$event->setAbTestResults()`` is where you set the actual A/B test results. More details are in the code example below.

A/B Testing Example
^^^^^^^
Copy link
Member

Choose a reason for hiding this comment

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

Heading nesting needs fixing, and it must reach the end of the text for the heading.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants