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] web: format hours as hh:mm in the Graph view #164280

Open
wants to merge 1 commit into
base: 17.0
Choose a base branch
from

Conversation

sami-odoo
Copy link
Contributor

@sami-odoo sami-odoo commented May 3, 2024

Before this Commit:
The Graph view was not adaptable to the use of widgets, leading to issues with
the representation of time. Specifically, hours were shown as float values in
the graph view. For example, 5 hours and 30 minutes were displayed as 5.50
instead of the more intuitive hh:mm format. Although widgets like "float_time"
or "timesheet_uom" were available to format these values, they were ineffective
in the Graph view due to the architecture parser's limitations. This caused
confusion for users trying to interpret the time accurately.

After this Commit:
The Graph view is now adaptable to the use of widgets. This means that when a
widget is applied to format a field's value, the value will be displayed in the
specified format. For example, hours can now be shown in the hh:mm format
instead of as a float. This improvement also ensures that the formatted values
are reflected in the Y-axis (Ticks/Intervals) of the Graph view, enhancing the
user's ability to interpret the data accurately.

Task-3861721


I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo
Copy link
Contributor

robodoo commented May 3, 2024

Pull request status dashboard.

@C3POdoo C3POdoo added the RD research & development, internal work label May 3, 2024
@aab-odoo aab-odoo marked this pull request as ready for review May 17, 2024 06:05
@C3POdoo C3POdoo requested review from a team, juliusc2066 and BastienFafchamps and removed request for a team May 17, 2024 06:06
@@ -16,6 +16,12 @@ import { cookie } from "@web/core/browser/cookie";

const NO_DATA = _t("No data");

// Measures for which value should be converted into HH:MM format
export const HOURS_MEASURE_FIELDS = [
Copy link
Contributor

Choose a reason for hiding this comment

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

You can't hardcode a list of fields in the framework. We should consider using the widget="..." feature as we do in a lot of other view types. For instance

<graph>
    <field name="unit_amount" type="measure" widget="float_time"/>
</graph>

And use the specified widget to format the value in the tooltip

Copy link
Contributor

Choose a reason for hiding this comment

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

Note: the formatter could also be used to format values displayed on the Y axis of the graph

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @aab-odoo sir,

Thank you for the suggestion. Previously, we used float_time or timesheet_uom, but no changes were reflected because the graph arch parser did not support widgets. However, this PR makes it adaptable to widgets, so now changes are easily reflected.

But there is a case:
There is a view, e.g., id="view_project_task_graph". If we do not have hr_timesheet installed, then the measures for which we have to display values in hh:mm are displayed in just float type. However, once we install hr_timesheet, it inherits the above view and modifies it by adding new fields as measures with specific widgets. So then the values are displayed in hh:mm. What should be done in that case? Should I explicitly add fields as measures in the parent view with widget="float_time" or "timesheet_uom" or use some other approach?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hello @sami-odoo

You should set widget="float_time" in the parent arch, and it will be overridden (with xpath) when hr_timesheet is installed, which is fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

so you will have to modify the xpaths in project_task_view_graph in order to get widget="timesheet_uom" instead of widget="float_time"

@sami-odoo sami-odoo force-pushed the 17.0-convert-to-hh-mm-imp-sami branch 6 times, most recently from 28b98dd to 8c27761 Compare May 21, 2024 11:11
@sami-odoo sami-odoo force-pushed the 17.0-convert-to-hh-mm-imp-sami branch from 8c27761 to 6cec0ed Compare May 24, 2024 09:30
@C3POdoo C3POdoo requested review from a team and gawa-odoo and removed request for a team May 24, 2024 09:33
@sami-odoo sami-odoo force-pushed the 17.0-convert-to-hh-mm-imp-sami branch from 6cec0ed to af9e231 Compare May 24, 2024 09:38
Before this Commit:
The Graph view was not adaptable to the use of widgets, leading to issues with
the representation of time. Specifically, hours were shown as float values in
the graph view. For example, 5 hours and 30 minutes were displayed as 5.50
instead of the more intuitive hh:mm format. Although widgets like "float_time"
or "timesheet_uom" were available to format these values, they were ineffective
in the Graph view due to the architecture parser's limitations. This caused
confusion for users trying to interpret the time accurately.

After this Commit:
The Graph view is now adaptable to the use of widgets. This means that when a
widget is applied to format a field's value, the value will be displayed in the
specified format. For example, hours can now be shown in the hh:mm format
instead of as a float. This improvement also ensures that the formatted values
are reflected in the Y-axis (Ticks/Intervals) of the Graph view, enhancing the
user's ability to interpret the data accurately.

Task-3861721
@sami-odoo sami-odoo force-pushed the 17.0-convert-to-hh-mm-imp-sami branch from af9e231 to eaa5351 Compare May 24, 2024 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants