-
Notifications
You must be signed in to change notification settings - Fork 10.8k
[IMP] companies: update digest emails #15672
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
|
Core CI + local review checks seem good, opening for review (triggering team review assignment and runbot instance) |
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 @nikibrown 💪
I only caught 2 small things, and left a number of other comments for your consideration.
I would minimally address what might be an unnecessary screenshot by using git rm command, as well as the unnecessary :align: tags under all of the images.
Everything else is related to the other list item details listed in the task, regarding the accuracy and usefulness of the content itself (optional).
Please let me know when this is ready for another look and I'll get right on it.
content/applications/general/companies/digest_emails/oob-periodic-digest-kpis-screenshot.png
Outdated
Show resolved
Hide resolved
|
|
||
| .. _digest-emails/custom-emails: | ||
|
|
||
| Create digest emails |
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 majority of this section is just duplicate content from what's on line 38. Wondering if the two can be combined so it's one solid section early in the doc?
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.
@StraubCreative I updated the PR based on the comments except this one. Don't feel like I know enough about the writing style to combine these sections.
a5de5a8 to
c28f59c
Compare
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.
Looks good 😎
Congrats on completing your first PR!
I have an optional fix below-- merge whenever you're ready and checks pass ✅
...
@robodoo delegate+
| app --> Emails section --> Digest Email field`. Then, select :guilabel:`Your Odoo Periodic Digest`, | ||
| and click on the :guilabel:`↗️ (External link)` icon, next to the drop-down menu selection. |
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.
optional: caught another unicode
I think it would be like this since we tend to favor contextual label names these days vs. the objective icon name
| app --> Emails section --> Digest Email field`. Then, select :guilabel:`Your Odoo Periodic Digest`, | |
| and click on the :guilabel:`↗️ (External link)` icon, next to the drop-down menu selection. | |
| and click on the :icon:`oi-arrow-right` :guilabel:`(internal link)` icon, next to the drop-down menu | |
| selection. |
|
@robodoo r+ |

Task