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

[IMP] stock_by_warehouse: Enhance stock visibility for warehouse-Wizard T#79454 #1685

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

TeposteAJ
Copy link
Contributor

@TeposteAJ TeposteAJ commented Oct 21, 2024

To enhance user comprehension by improving the visibility of stock levels
across different warehouses, especially when creating a sale and checking
stock availability. This feature is particularly useful for managing stock
across multiple locations. Additionally, conditional help tooltips have
been added to assist in correctly interpreting the values, and translation
terms have been updated.

Requisition

Change the aesthetics of the Stock by Warehouse Wizard, which is optionally displayed in sales orders. Only the fields of interest need to be kept, and the structure should be made easier to understand.
Related to: vauxoo/typ#530

Sale order form view (Stock by Warehouse Wizard) :

  • Before development you have the following default view:
    image

  • After development you have the following default view:
    image

@TeposteAJ
Copy link
Contributor Author

Hi @luisg123v could you review it ? , please

@luisg123v luisg123v changed the title [IMP] stock_by_warehouse: Enhance stock visibility for warehouse-Wizard [IMP] stock_by_warehouse: Enhance stock visibility for warehouse-Wizard T#79454 Oct 21, 2024
Comment on lines 102 to 105
</td>
</tr>
</t>
</t>
Copy link
Contributor

Choose a reason for hiding this comment

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

There are problems in the indentation

#, python-format
msgid "Incoming stock may not cover or will barely meet demand."
msgstr ""
"Es posible que el stock entrante no cubra o apenas satisfaga la demanda."
Copy link
Contributor

Choose a reason for hiding this comment

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

inventario instead of stock

Comment on lines 96 to 88
t-att-class="
line.virtual > 3 ? '' :
(line.virtual > 0 ? 'text-warning' : 'text-danger')"
>
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation

Suggested change
t-att-class="
line.virtual > 3 ? '' :
(line.virtual > 0 ? 'text-warning' : 'text-danger')"
>
t-att-class="
line.virtual > 3 ? '' :
(line.virtual > 0 ? 'text-warning' : 'text-danger')"
>

@rolandojduartem
Copy link
Contributor

Functionally LGTM

@TeposteAJ TeposteAJ force-pushed the 17.0-Imp_Wizard_quants-TeposteAJ branch from 097e90b to 65c8b9f Compare October 22, 2024 00:33
@TeposteAJ TeposteAJ force-pushed the 17.0-Imp_Wizard_quants-TeposteAJ branch 3 times, most recently from 54c758e to d9640c8 Compare October 22, 2024 16:53
@TeposteAJ TeposteAJ force-pushed the 17.0-Imp_Wizard_quants-TeposteAJ branch from d9640c8 to 306f91a Compare October 23, 2024 00:36
@TeposteAJ TeposteAJ requested a review from luisg123v October 23, 2024 00:36
@TeposteAJ TeposteAJ force-pushed the 17.0-Imp_Wizard_quants-TeposteAJ branch 2 times, most recently from d0d1521 to 52b567a Compare October 23, 2024 15:33
<t t-if="line.virtual gt 0" t-set="saleable_qty_title_message">
<t> This quantity is available for immediate sale and delivery.</t>
<t t-out="'\n'" />
<t> This represents on hand quantity minus the reserved stock.</t>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just one more matter:

Is it accurate to mention reserved quantity? Be aware, starting from v17.0, the concept of reserved quantity doesn't exist anymore, not as handled in previous versions anyway, see odoo/odoo@7dda6bb9 for more info.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ready!
:o This is important! tks c: @luisg123v

@TeposteAJ TeposteAJ force-pushed the 17.0-Imp_Wizard_quants-TeposteAJ branch from 52b567a to 832bc2b Compare October 24, 2024 15:54
@TeposteAJ TeposteAJ requested a review from luisg123v October 24, 2024 17:28
@TeposteAJ TeposteAJ force-pushed the 17.0-Imp_Wizard_quants-TeposteAJ branch from 832bc2b to 8afba78 Compare October 24, 2024 22:08
</td>
<td style="text-align: center;">
<t t-if="line.virtual gt 0" t-set="saleable_qty_title_message">
<t> This quantity is available for immediate sale and delivery.</t>
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for not noticing it before:

Suggested change
<t> This quantity is available for immediate sale and delivery.</t>
<t>This quantity is available for immediate sale and delivery.</t>

Same below.

Copy link
Contributor Author

@TeposteAJ TeposteAJ Oct 25, 2024

Choose a reason for hiding this comment

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

@luisg123v no worries! No need to apologize 😉. Done!

@luisg123v
Copy link
Contributor

Hi @rolandojduartem,

This in general LGTM. Could you review again, please?

Regards,

@TeposteAJ TeposteAJ force-pushed the 17.0-Imp_Wizard_quants-TeposteAJ branch from 8afba78 to 51664c0 Compare October 25, 2024 15:37
@TeposteAJ TeposteAJ requested a review from luisg123v October 25, 2024 22:19
@TeposteAJ TeposteAJ force-pushed the 17.0-Imp_Wizard_quants-TeposteAJ branch 2 times, most recently from 452f43d to dbd590f Compare October 26, 2024 00:35
@rolandojduartem
Copy link
Contributor

I like the new style, but in mobile is buggy

Please, in this line https://github.com/Vauxoo/addons-vauxoo/blob/8dc846c4/stock_by_warehouse/static/src/components/warehouse/warehouse_field.esm.js#L78

Modify it to position: localization.direction === "rtl" || isMobileOS() ? "bottom" : "right",

You could import isMobileOS as
import { isMobileOS } from "@web/core/browser/feature_detection";

Currently, you could not see all the information

Screenshot from 2024-10-25 21-39-23

but, the proposed solution will position the popover at the botton of the field and you can check the information easily.

Screenshot from 2024-10-25 21-40-02

</td>
<td style="text-align: center;">
<t t-if="line.virtual gt 0" t-set="saleable_qty_title_message">
<t>This quantity is available for immediate sale and delivery.</t>
Copy link
Contributor

Choose a reason for hiding this comment

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

So, @luisg123v, do we want these messages here?

Copy link
Contributor

Choose a reason for hiding this comment

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

@luisg123v just this comment for me, if you are OK, so LGTM

Copy link
Contributor

Choose a reason for hiding this comment

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

@rolandojduartem yes, it's expected. It's the improvement implemented here, showing not only the quantities but also their meaning.

To enhance user comprehension by improving the visibility of stock levels
across different warehouses, especially when creating a sale and checking
stock availability. This feature is particularly useful for managing stock
across multiple locations. Additionally, conditional help tooltips have
been added to assist in correctly interpreting the values, and translation
terms have been updated.
@TeposteAJ TeposteAJ force-pushed the 17.0-Imp_Wizard_quants-TeposteAJ branch from dbd590f to f38c703 Compare October 28, 2024 16:37
@TeposteAJ
Copy link
Contributor Author

@rolandojduartem Tks u 4 the improvements, the changes have been added c:

@rolandojduartem
Copy link
Contributor

LGTM 👍 @luisg123v

Copy link
Contributor

@luisg123v luisg123v left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@luisg123v luisg123v merged commit 23c94e6 into Vauxoo:17.0 Oct 29, 2024
3 checks passed
@luisg123v luisg123v deleted the 17.0-Imp_Wizard_quants-TeposteAJ branch October 29, 2024 22:11
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.

3 participants