-
Notifications
You must be signed in to change notification settings - Fork 0
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
[#45] Destruction list detail view #54
Conversation
fa86858
to
b19e3f0
Compare
completed: "Completed", | ||
}; | ||
|
||
function formatUser(user: User) { |
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.
The landing page doesn't do this either right now, but I think we should think about combining some of the things here like the STATUS_MAPPING
and the formatUser()
since the landing page also uses them (and possibly others later on). For example a function formatUser()
that takes in some options to customize the output would be nice, and a central place to get the status mappings from.
I think we can ignore it for now though
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 moved the status mappings to a constants.ts files and the format user function to a utils file. Then it can be more easily reused by the landing page. Then we can expand the options once we have a better idea of what we need!
|
||
export const destructionListDetailLoader = loginRequired( | ||
async ({ request, params }: ActionFunctionArgs) => { | ||
if (typeof params.id === "undefined") return; |
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.
Just to make sure, what happens if we hit this return?
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.
The UI crashes, because the loader didnt return anything 😄 I changed it to make it return an empty dict, and then the DestructionListDetailPage
renders a (temporary) 404 page.
<DataGrid | ||
title="Zaakdossiers" | ||
count={allZaken.count} | ||
objectList={allZaken.results as unknown as AttributeData[]} |
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 is this type assertion (and similars in the render) needed? Don't we already type assert it higher up the tree at line 30?
const { zaken: allZaken, zaaktypeChoices } = useLoaderData() as DestructionListDetailContext;
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 DestructionListDetailContext
they are defined as PaginatedZaken
and allZaken.results
is then Zaak[]
. I added it to make it stop complaining about it :S
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.
Hmm yeah, the DataGrid
component expects AttributeData[]
array I guess, a bit sad that these things aren't compatible, should be ok then haha
20040c3
to
8d983e8
Compare
8d983e8
to
433eca1
Compare
433eca1
to
02b30b1
Compare
02b30b1
to
12da0fa
Compare
12da0fa
to
ef39a58
Compare
Fixes #45