[FIX] hr_holidays: dynamic accrual days #4904
Conversation
|
This PR targets the un-managed branch odoo-dev/odoo:master-hr-onboarding-haabo, it needs to be retargeted before it can be merged. |
hazemayman1
force-pushed
the
master-hr-onboarding-ux_accrual-haabo
branch
from
4329ea4 to
3207f98
Compare
Changed the {day} dropdown to adapt to the selected month. It was previously set to 31 for all months which allowed the user to select the 31st on a 28 or 30 day month for example.
task-5392293
hazemayman1
force-pushed
the
master-hr-onboarding-ux_accrual-haabo
branch
from
3207f98 to
d7d62df
Compare
Mahmoudk3m
left a comment
Mahmoudk3m
left a comment
There was a problem hiding this comment.
Thanks for your work 🚀
Just a few comments.
| let date = new Date(2024, selectedMonth, 0); | ||
| let days = date.getDate(); |
There was a problem hiding this comment.
| let date = new Date(2024, selectedMonth, 0); | |
| let days = date.getDate(); | |
| const date = new Date(2024, selectedMonth, 0); | |
| const days = date.getDate(); |
Use const instead of let where values don't change:
| const selectedMonth = this.props.record.data[this.props.selectedMonth]; | ||
| let date = new Date(2024, selectedMonth, 0); | ||
| let days = date.getDate(); | ||
| let newChoicesList = Array.from({length: days}, (_, i) => [(i + 1).toString(),(i + 1).toString()]) |
There was a problem hiding this comment.
| let newChoicesList = Array.from({length: days}, (_, i) => [(i + 1).toString(),(i + 1).toString()]) | |
| let newChoicesList = Array.from({length: days}, (_, i) => [(i + 1).toString(), (i + 1).toString()]) |
missing space after comma
|
|
||
| get options() { | ||
| const selectedMonth = this.props.record.data[this.props.selectedMonth]; | ||
| let date = new Date(2024, selectedMonth, 0); |
There was a problem hiding this comment.
I would add a comment to explain why I used 2024 specifically, because it might be a bit confusing.
|
|
||
|
|
||
| get options() { | ||
| const selectedMonth = this.props.record.data[this.props.selectedMonth]; |
There was a problem hiding this comment.
If selectedMonth is undefined or invalid, this could fail silently.
There was a problem hiding this comment.
If a user has selected "31" for the day and then changes the month to one with only 30 days, what happens? 🤔
Changed the {day} dropdown to adapt to the selected month. It was previously set to 31 for all months which allowed the user to select the 31st on a 28 or 30 day month for example.
task-5392293