Skip to content
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-4750 create the page with all elements filters table for inbound movements #3503

Conversation

drodzewicz
Copy link
Collaborator

No description provided.

- add new button variant primary-outline
- remove duplicate button variant classes
- add endpoint for getting shipment status codes
- save shipment status codes in redux global state
- center tooltip on cell value and not value container
- add default value to TableCell component
- Creat Header, Filter, Table and List component for StockMovements inbound
- add shipment status description translations
@drodzewicz drodzewicz self-assigned this Sep 28, 2022
@drodzewicz drodzewicz marked this pull request as draft September 28, 2022 15:47
@drodzewicz drodzewicz marked this pull request as ready for review September 28, 2022 17:27
@drodzewicz drodzewicz force-pushed the OBPIH-4750-create-the-page-with-all-elements-filters-table-for-inbound-movements branch from 3632297 to 9bd0c19 Compare September 28, 2022 17:35
@awalkowiak awalkowiak merged commit 49b523d into feature/ui-redesign Sep 28, 2022
@awalkowiak awalkowiak deleted the OBPIH-4750-create-the-page-with-all-elements-filters-table-for-inbound-movements branch September 28, 2022 22:12
}
form.reset(initialValues);
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

it wasn't necessary (comment below)

searchFieldPlaceholder="Search by order number of description"
filterFields={filterFields}
defaultValues={{
...filterParams,
Copy link
Collaborator

Choose a reason for hiding this comment

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

you could just make defaultValues as

destination: {
     id: currentLocation.id,
     value: currentLocation.id,
     name: currentLocation.name,
     label: currentLocation.name,
   },

and you wouldn't have to do this onClear prop. Assigning destination to filterParams as default state disables you from assigning it again if the filterParams change, so I probably know, why you made this onClear, but if you had passed destination as defaultValues here, you wouldn't have had to do this onClear.



useEffect(() => {
if (!isShipmentStatusesFetched) fetchStatuses();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would add !isShipmentStatusesFetched || shipmentStatuses.length === 0 in case somehow it fetched empty list and we would want to refetch it (low possibiltiy to happen)

name: props.currentLocation.name,
label: props.currentLocation.name,
},
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

As commented above - filterParams should stay {}, but defaultValues passed to FilterForm should be this.

// If filterParams change, refetch the data with applied filters
useEffect(() => {
fireFetchData();
if (!isShipmentStatusesFetched) fetchStatuses();
Copy link
Collaborator

Choose a reason for hiding this comment

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

as above, I would add additional || shipmentStatuses.length === 0 for safety.

{
sort: state.sorted[0].id,
order: state.sorted[0].desc ? 'desc' : 'asc',
} : undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume anyway backend sorts by something by default, but if not, add here sorting by date by default maybe?

setTotalData(res.data.totalCount);
setTableData(res.data.data);
// Store currently used params for export case
setCurrentParams(params);
Copy link
Collaborator

Choose a reason for hiding this comment

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

as discussed yesterday, maybe do one object-state, so you don't have to make multiple setStates where each of them re-renders component?

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.

None yet

3 participants