[IMP] Sales: down payment update #15498
Conversation
|
|
jero-odoo
left a comment
jero-odoo
left a comment
There was a problem hiding this comment.
Hey @theRealThagomizer looks good to me! A few quick notes, and some general comments about images, otherwise this is good for the next round
| the invoicing policy is configured to require delivery before invoicing. | ||
| If an :guilabel:`Invalid Operation` error appears, double-check that the :doc:`invoicing policy | ||
| <invoicing_policy>` is configured correctly. In some cases, for example, the invoicing policy is | ||
| configured to require delivery before invoicing. |
There was a problem hiding this comment.
Instead of "before invoicing" I would consider "before sending an invoice" or "before invoicing can be processed"
Not necessary, but its slightly more clear
| page. | ||
| .. important:: | ||
| To change or adjust the income account attached to down payments, the **Accounting** app must be | ||
| installed. With the *Accounting* app installed, the :guilabel:`Accounting` column becomes |
There was a problem hiding this comment.
| installed. With the *Accounting* app installed, the :guilabel:`Accounting` column becomes | |
| installed. With the **Accounting** app installed, the :guilabel:`Accounting` column becomes |
| .. image:: down_payment/income-account.png | ||
| :align: center | ||
| :alt: How to modify the income account link to down payments. | ||
| In the :guilabel:`Search: Account` form, a different account be chosen from the list of pre-existing |
There was a problem hiding this comment.
| In the :guilabel:`Search: Account` form, a different account be chosen from the list of pre-existing | |
| In the :guilabel:`Search: Account` form, a different account can be chosen from the list of pre-existing |
| invoice changes the status from :guilabel:`Draft` to :guilabel:`Posted`. It also reveals a new | ||
| series of buttons at the top of the page. | ||
|
|
||
| .. image:: down_payment/draft-invoice-sample.png |
There was a problem hiding this comment.
I know this wasn't part of the update, but it looks like there were changes to this view (see below). It is probably worth it to do a quick review of the images and make sure they are up to date for 18 (and also do the image capture again without the markup when possible)
theRealThagomizer
force-pushed
the
18.0-sales-down-payment-correction-thjud
branch
from
e4ebcc1 to
6acfc46
Compare
|
Thanks for the review, @jero-odoo! I used your feedback as a jumping off point to go through the doc and do a more thorough rewrite and replacement of outdated images. If you'd like to take another look at it, let me know. Otherwise, I'll tag in Zach. |
jero-odoo
left a comment
jero-odoo
left a comment
There was a problem hiding this comment.
The images are much better this time @theRealThagomizer ! There is an error when you run make fast, and is causing the failure of the ci check. I mentioned below how to fix it, but let me know if you run into issues. Thanks!
| In the :guilabel:`Search: Account` form, a different account can be chosen from the list of | ||
| pre-existing accounts. A new account can also be created by clicking the :guilabel:`New` button. | ||
|
|
||
| .. image:: down_payment/income-account.png |
There was a problem hiding this comment.
The documentation check is failing becase of this image tag (it is also showing up as an error when you run make fast). You need to make sure that the second line of the alt tag is aligned correctly (see image)
Let me know if you need help with this!
theRealThagomizer
force-pushed
the
18.0-sales-down-payment-correction-thjud
branch
from
6acfc46 to
7da1aa3
Compare
|
Thanks, Jess! I swear I ran both make fast and make review several times. Maybe I thought I solved a different problem and caused that one in the process without double-checking after? Regardless, thanks for the second review! @StraubCreative, got a file with some shiny new images for you! |
StraubCreative
left a comment
StraubCreative
left a comment
There was a problem hiding this comment.
Looks good, can merge when ready 🚀
...
@robodoo delegate+
There was a problem hiding this comment.
General feedback:
The previous screenshot that was deleted is actually a good example of capturing the UI in HD. Even though it has smaller dimensions, the contents are a bit more legible.
If you're not doing so already, consider zooming in on your shots to ~125% (before the mobile breakpoint activates), resizing your browser window, or (nicely) hard cropping just the important parts that are contextual to the written text.
Refer to the numbered list items here in the content guidelines.
Co-authored-by: Jess Rogers <jero@odoo.com>
theRealThagomizer
force-pushed
the
18.0-sales-down-payment-correction-thjud
branch
from
7da1aa3 to
6996426
Compare
|
Thanks, @StraubCreative! I dove back in one last time and got better screen grabs of the images and made some really minor content tweaks. I'll get this merged now. Big thanks to you and @jero-odoo for the keen eyes! |
|
@robodoo r+ |
closes #15498 Signed-off-by: Thomas Jude Cavazos (thjud) <thjud@odoo.com> Co-authored-by: Jess Rogers <jero@odoo.com>
4 participants
Hiya, Jess! I hope you have/had a great holiday weekend. Got a PR here that updates the language on the down payments page from talking about down payments being a kind of product to having them be their own thing. The page in general seems kind of outdated (it at least needs new pictures), so I'll jump back into it in a future PR, but for now, I just wanted to address this task: https://www.odoo.com/odoo/action-4043/4723773
Thank you!
This 18.0 PR can be FWP up to master.