-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[IMP] marketing: RST format upgrades for marketing automation v14 #4320
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
Conversation
ksc-odoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samueljlieber would you mind giving this a quick technical review? I made some RST updates/changes to reflect the formatting decisions made during our recent Writer's Workshop meeting. Thanks!
Also, I'll be continuing to do this for all published Marketing docs, so if you think there's a better/easier way to do this - as opposed to a slurry of PRs - just let me know, and I'll make it happen. Thanks!
e55bd3f to
5fbef09
Compare
Super confused about all of the errors tbh. Will try a few things... |
5fbef09 to
91cbe6d
Compare
samueljlieber
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ksc-odoo 👋
I've made some technical changes here. Please review and let me know if you approve or have any changes. I will push my changes up as a commit after this review 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use menuselection here because we are directing the user.
| To create a new automated marketing campaign, open the :guilabel:`Marketing Automation` app, | |
| To create a new automated marketing campaign, open the :menuselection:`Marketing Automation` app, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling correction.
| futher narrow down the target recipients/audience for the marketing automation campaign. | |
| further narrow down the target recipients/audience for the marketing automation campaign. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Icons in guilabels can't have a space after the first backtick and before the emoji, the guilabel wont format correctly. Also, include the parenthesis text within the guilabel too like this:
| To add another node, click the :guilabel:` ➕ ` (plus sign) icon to the right of the filtering | |
| rule. To add a branch of multiple nodes at the same time, click the :guilabel:` ... ` (ellipses) | |
| To add another node, click the :guilabel:`➕ (plus sign)` icon to the right of the filtering | |
| rule. To add a branch of multiple nodes at the same time, click the :guilabel:`⋯ (ellipses)` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating to use global relative path rather than local relative path.
| For further information on filters, refer to :doc:`this documentation page <target_audience>`. | |
| For further information on filters, refer to :doc:`this documentation page | |
| </applications/marketing/marketing_automation/getting_started/target_audience>`. |
91cbe6d to
afab01a
Compare
|
Implemented my technical changes in afab01a. |
@samueljlieber I fully approve of the changes - thanks for the in-depth review, as always |
|
Hi @StraubCreative this PR is ready for your review! |
StraubCreative
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ksc-odoo
Couple small changes. Everything else looked good.
Tag me again when these are addressed and we can merge 👍
Remind me: should these changes be forward ported to 15 and 16 (it's the same content right)?
Later:
At some point let's talk about consolidating some of these docs so they're each a more complete use case.
As is, this doc only covers the macro settings of a MA campaign, however, these settings aren't enough to launch an MA campaign; a partial walk-through of the UI isn't as useful as, say, a doc that builds a campaign from start to finish and clicks launch.
cc: @samueljlieber
content/applications/marketing/marketing_automation/getting_started/first_campaign.rst
Outdated
Show resolved
Hide resolved
| @@ -1,59 +1,70 @@ | |||
| ==================== | |||
| Marketing Automation | |||
| Marketing automation | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a different title.
As it is the TOC navigation is Marketing Automation --> Getting Started --> Marketing Automation
I'm thinking this doc can be called Getting started, which funny enough @ksc-odoo might have been the same or similar title you gave it originally on the initial PR 😄
Also do we need the subsections Getting Started and Advanced?
Can we just have all 5 docs listed under Marketing --> Marketing Automation, like how Events is structured?
cc: @samueljlieber
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the title to "Getting started" (per your suggestion)
And, honestly, in my opinion, no, I don't think we need the subsections - I was just working off the previously established format. I believe we can have all 5 docs listed under Marketing --> Marketing Automation, just like the Events structure.
cc: @samueljlieber
Re: if it should be forward ported to 15 and 16.... I'd say "yes" because this meets the current formatting guidelines, and would only need minimal adjustments (perhaps an updated screenshot/removal of "save" mentions, etc) - which I can take care of in the near future - but this version is better than the current doc for Marketing Automation, in my opinion @samueljlieber @StraubCreative Also, I made the necessary adjustments, sent a fresh, fully-updated RST file to @samueljlieber and he should be able to push it (and forward port it) soon. If there's anything else you need from me, just let me know. thanks! |
afab01a to
69fa638
Compare
|
Hi @ksc-odoo, I implemented your changes in 69fa638 and I feel this PR is good to go! I agree with @StraubCreative's comment about restructuring Marketing Automation to be more like Events. @ksc-odoo lets create a task for this 🙂 |
StraubCreative
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving so we can merge the good progress we have already.
Can work on small stuff later as well as restructure.
In the future I don't want to see creative parenthesis use or idioms 😉
@robodoo r+
|
|
||
| - :guilabel:`Templates`: represents the number of pre-configured mail templates being used in this | ||
| particular campaign. (Templates can always be created on-the-fly as well). | ||
| particular campaign. (Templates can always be created on-the-fly, as well). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A sentence that's in entirely in parenthesis is not grammatically correct.
Avoid idioms like "on-the-fly"
| particular campaign. (Templates can always be created on-the-fly, as well). | |
| particular campaign. (Templates can always be created on-the-fly, as well). |
| :align: center | ||
| :alt: A dashboard showing the creation of a new marketing automation campaign in Odoo. | ||
|
|
||
| **Smart buttons** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be an h3 heading?
| - :guilabel:`Participants`: represents the number of contacts that have directly participated in | ||
| this campaign. | ||
|
|
||
| **Fields** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
heading
closes #4320 Signed-off-by: Zachary Straub (zst) <zst@odoo.com>
closes #4320 Signed-off-by: Zachary Straub (zst) <zst@odoo.com>
v14 marketing automation > getting started > first campaign RST formatting updates/upgrades
made necessary formatting changes to adhere to rst decisions made during writer's workshop meeting
This PR should be FWP to 15.0 & 16.0
Project Task: https://www.odoo.com/web#id=3329135&cids=3&menu_id=4720&action=333&active_id=3835&model=project.task&view_type=form