-
Notifications
You must be signed in to change notification settings - Fork 4
rework limits modification #3278
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
Conversation
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
…y from CurrentLimitsFormInfos Signed-off-by: Mathieu DEHARBE <[email protected]>
export const TEMPORARY_LIMIT_VALUE = 'value'; | ||
export const TEMPORARY_LIMIT_MODIFICATION_TYPE = { | ||
MODIFY: 'MODIFY', | ||
MODIFY_OR_ADD: 'MODIFY_OR_ADD', // if the limit exists it is modified, if not it is created |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe should rename it MODIFY OR ELSE ADD, it s easier to understand at first sight
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(in the back also)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right it would be clearer. Sadly I already merged MODIFY_OR_ADD for the operational limits groups equivalent. If I change one I should change both.
const { fields, append, remove } = useFieldArray({ name: arrayFormName }); | ||
const { fields, append } = useFieldArray({ name: arrayFormName }); | ||
const [hoveredRowIndex, setHoveredRowIndex] = useState(-1); | ||
const { setValue, getValues } = useFormContext(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not using useFieldArray methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to but I failed to do it.
fields
only return a record of keys/default values. Is there something better thangetValues
and this ?update
would change the whole object at the index instead of just the DELETION_MARK part (I could combine it with a getValues but then it lose its point).
Do you have a more precise idea ?
|
||
modificationLimitsGroups = modificationLimitsGroups.map((formLimitsGroup: OperationalLimitsGroup) => { | ||
return modificationLimitsGroupsForm.map((limitsGroupForm: OperationalLimitsGroupFormInfos) => { | ||
const modificationType: string = LIMIT_SETS_MODIFICATION_TYPE.MODIFY_OR_ADD; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe delete this var and include the value directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I see its the same elsewhere...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK Done.
Co-authored-by: Florent MILLOT <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
…rework-limit-modification
|
No description provided.