Skip to content

[IMP] hr_holidays: cap carryover day selection based on month - #5239

Open
amgom-odoo wants to merge 1 commit into
master-hr-onboarding-amgomfrom
master-time-off-accrual-ux-amgom
Open

[IMP] hr_holidays: cap carryover day selection based on month#5239
amgom-odoo wants to merge 1 commit into
master-hr-onboarding-amgomfrom
master-time-off-accrual-ux-amgom

Conversation

@amgom-odoo

Copy link
Copy Markdown

Introduce a new widget to cap the carryover day selection based on the selected month

Task: 6459855

@robodoo

robodoo commented Aug 11, 2026

Copy link
Copy Markdown

This PR targets the un-managed branch odoo-dev/odoo:master-hr-onboarding-amgom, it needs to be retargeted before it can be merged.

Introduce a new widget to cap the carryover day selection based on the selected month

Task: 6459855
@amgom-odoo
amgom-odoo force-pushed the master-time-off-accrual-ux-amgom branch from cd44b77 to d8c84d2 Compare August 12, 2026 11:14

@lebm-odoo lebm-odoo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hellooo, nice widget. Some little points otherwise it's already good 😄

Comment on lines +18 to +21
const monthMaxDays = {
"1": 31, "2": 29, "3": 31, "4": 30, "5": 31, "6": 30,
"7": 31, "8": 31, "9": 30, "10": 31, "11": 30, "12": 31
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe you can try to make it prettier, something should exists to get the last day of a given month without hardcoding it 👀 (using luxon or standard js Date 🤷‍♂️)

Comment on lines +32 to +45
supportedOptions: [
...(selectionField.supportedOptions || []),
{ name: "month_field", type: "string" }
],

extractProps: (fieldInfo, dynamicInfo) => {
const props = selectionField.extractProps(fieldInfo, dynamicInfo);

if (fieldInfo.options && fieldInfo.options.month_field) {
props.monthField = fieldInfo.options.month_field;
}

return props;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's good, you can use attrs too. options are more used to set multiples values that will be evaluated. and attrs will be just string. so in your case, an attr is more adapted here I think 👀
And don't forget to add fieldDependencies, so that you'll not be forced to add the field in the view in invisible to be able to retreive it in the js record. 😄

export class CarryoverDaySelection extends SelectionField {
props = useProps();

get options() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

it's a good practice to add

/**
 * @override
 * /
 get options(){

As it comes from SelectionField

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