Skip to content

Conversation

ravb-odoo
Copy link
Contributor

This PR improves the ux and functionality for both the contract and work entry form views.

task-4334973

@robodoo
Copy link
Contributor

robodoo commented Jan 20, 2025

Pull request status dashboard

@ravb-odoo ravb-odoo force-pushed the master-hr-payroll-ux-improvement-ravb branch from 0cdd086 to 2929461 Compare January 20, 2025 09:26
@C3POdoo C3POdoo added the RD research & development, internal work label Jan 20, 2025
widget="state_selection" readonly="0"/>
</h1>
<h2>
<h2 class="d-none">
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't invisible="1" working here?

Copy link
Contributor Author

@ravb-odoo ravb-odoo Feb 10, 2025

Choose a reason for hiding this comment

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

We can't use invisible because it removes the entire <h2> tag from the HTML structure, causing some tours and test cases to fail when searching for it. Instead, using d-none will hide it from the UI while keeping it in the structure, ensuring that the tours and tests still pass.

Copy link
Contributor

Choose a reason for hiding this comment

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

And couldn't we adapt the tour instead ?

@ravb-odoo ravb-odoo force-pushed the master-hr-payroll-ux-improvement-ravb branch 3 times, most recently from bc40377 to fb6e214 Compare February 17, 2025 05:47
@ravb-odoo ravb-odoo force-pushed the master-hr-payroll-ux-improvement-ravb branch 4 times, most recently from 1ed6319 to e2d0a00 Compare February 18, 2025 12:07
Copy link
Contributor

@Flotchet Flotchet left a comment

Choose a reason for hiding this comment

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

Hello thanks for your work first round of review

Comment on lines 1 to 5
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like a really bad way to do this hardcoding stuff especially position of the the template layering is not the way to go it's basically killing a fly with a bazooka

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that you can just move the ribbon position in the template

Copy link
Contributor

Choose a reason for hiding this comment

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

what is the point of having a non empty header that cannot ever be visible?

Comment on lines 189 to 191
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the point of a none display unempty stuff ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to use d-none because it prevents extra space from appearing between the name and employee_id fields in contract form view. Using invisible causes some tours and test cases to fail when searching for the field. Instead, d-none hides it from the UI while keeping it in the structure, ensuring that the tours and tests continue to pass.

Copy link
Contributor

Choose a reason for hiding this comment

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

120c max

Copy link
Contributor

Choose a reason for hiding this comment

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

120c max

@ravb-odoo ravb-odoo force-pushed the master-hr-payroll-ux-improvement-ravb branch 5 times, most recently from 3e87649 to 928a65e Compare March 3, 2025 12:31
@ravb-odoo ravb-odoo force-pushed the master-hr-payroll-ux-improvement-ravb branch from 928a65e to 7d7bdb5 Compare March 7, 2025 10:27
Copy link
Contributor

@tivisse tivisse left a comment

Choose a reason for hiding this comment

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

Small review.

Thanks for your work.

widget="state_selection" readonly="0"/>
</h1>
<h2>
<h2 class="d-none">
Copy link
Contributor

Choose a reason for hiding this comment

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

And couldn't we adapt the tour instead ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
invisible="contracts_count &lt; 2"
invisible="contracts_count &lt;= 1"

@ravb-odoo ravb-odoo force-pushed the master-hr-payroll-ux-improvement-ravb branch 4 times, most recently from 11f9fa6 to d430b25 Compare March 19, 2025 05:51
@tivisse tivisse marked this pull request as ready for review March 24, 2025 13:30
This PR improves the ux and functionality for both the contract and work entry
form views.

task-4334973
@tivisse tivisse force-pushed the master-hr-payroll-ux-improvement-ravb branch from d430b25 to d67c0d6 Compare March 24, 2025 13:42
@tivisse
Copy link
Contributor

tivisse commented Mar 24, 2025

@robodoo r+

@C3POdoo C3POdoo requested a review from a team March 24, 2025 13:47
robodoo pushed a commit that referenced this pull request Mar 24, 2025
This PR improves the ux and functionality for both the contract and work entry
form views.

task-4334973

closes #194330

Related: odoo/enterprise#77409
Related: odoo/upgrade#7089
Signed-off-by: Yannick Tivisse (yti) <yti@odoo.com>
@robodoo robodoo closed this Mar 24, 2025
@robodoo robodoo added the 18.3 label Mar 24, 2025
@tivisse tivisse deleted the master-hr-payroll-ux-improvement-ravb branch March 25, 2025 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

18.3 RD research & development, internal work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants