-
-
Notifications
You must be signed in to change notification settings - Fork 406
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
OBPIH-6372 full outbound import confirm page frontend #4660
OBPIH-6372 full outbound import confirm page frontend #4660
Conversation
546658c
to
28468a9
Compare
There are some commits from crowding OBPIH-6328 and OBPIH-6365 most likely because #4656 is not rebased with develop. |
28468a9
to
93dc469
Compare
TrComponent={TableRow} | ||
getTrProps={(state, rowInfo) => ({ | ||
row: rowInfo?.row, | ||
error: _.get(errors, `${rowInfo?.index}`, undefined), |
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 errors?.rowInfo?.index
?
&.rt-invalid { | ||
.rt-td { | ||
background: var(--red-100); | ||
} | ||
.invalid-cell { | ||
color: var(--red-500) | ||
} | ||
} |
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.
Please add new lines
&.rt-invalid { | |
.rt-td { | |
background: var(--red-100); | |
} | |
.invalid-cell { | |
color: var(--red-500) | |
} | |
} | |
&.rt-invalid { | |
.rt-td { | |
background: var(--red-100); | |
} | |
.invalid-cell { | |
color: var(--red-500) | |
} | |
} |
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 think this component was removed in Kacper's PR
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.
👆
9f8c2f1
to
25ebbad
Compare
id: 'react.default.dateInput.placeholder.label', | ||
default: 'Select a date', | ||
}} | ||
errorMessage={errors.dateShipped?.message} |
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 think you missed showTimeSelect
prop.
</div> | ||
<div className="col-lg-3 px-2 pt-2 d-lg-block d-md-none" /> | ||
<div className="col-lg-3 px-2 pt-2 d-lg-block d-md-none" /> | ||
<div className="col-lg-3 col-md-6 px-2 pt-2"> |
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.
what are those two empty divs responsible for?
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.
Let me share a screenshot for how the confirm page looks like first
So there are supposed to be a space between two input, and these two empty divs just act as a placeholders empty spaces for the grid.
To answer your potential question "Why did you not use space-between?" it's because when using space-between it will not have the same effect and will result in these two fields be of different size than the ones above since they are going to be pushed to the smallest element size which is different to the auto-fit size of the grid elements.
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.
sounds good
{is(OutboundImportStep.DETAILS.key) && (<Step.Component {...detailsComponentProps} />)} | ||
{is(OutboundImportStep.CONFIRM.key) && (<Step.Component previous={previous} />)} | ||
</form> | ||
<form onSubmit={handleSubmit(onConfirmImport)}> |
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.
nice idea with having two separate forms, triggering the same handleSubmit
- I assume we will be able easily to manipulate whether we trigger the validation endpoint or the save (onSubmitStockMovementDetails would trigger the validation endpoint and the onConfirmImport would trigger the save)
3add7d9
to
403ea76
Compare
This PR should be merged after #4656 because I based my approach on Kacpers details step