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
wip - initial table load with list provider #212
Conversation
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.
keep going, its improving!
export type ParentResource = | ||
| ResourceType.USERS | ||
| ResourceType.GROUPS | ||
| ResourceType.APPLICATIONS | ||
| ResourceType.POLICIES; | ||
|
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.
Yay moved somewhere central!
const loadList = useCallback(async () => { | ||
if (resourceName) { | ||
const getList = RESOURCE_MAP[resourceName].getList; | ||
const newParams = { | ||
...currentListParams, | ||
sortField: getInitialSortField(resourceName), | ||
}; | ||
// TODO: there's still 2 api requests happening, because of params update i think. need to investigate | ||
const data = await getList(newParams); | ||
setListState(data); | ||
} | ||
}, [resourceName, currentListParams]); | ||
|
||
useEffect(() => { | ||
if (resourceName) { | ||
setCurrentResource(resourceName); | ||
setCurrentListParams((current) => ({ | ||
...current, | ||
sortField: getInitialSortField(resourceName), | ||
})); | ||
} | ||
}, [resourceName]); | ||
|
||
useEffect(() => { | ||
loadList(); | ||
}, [loadList]); |
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.
do we use loadList
anywhere other than this useEffect? if not, shouldnt we just define a use effect based on the state of [resourceName, currentListParams]
that immediately calls the function? It feels cumbersome to define it as a variable then just use it once.
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 as of right now it's not...but i will refresh my memory on what my plan was here!
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.
Actually i'm going to leave this as is, i have a follow-up branch that adds in the params/sorting etc so i can make the change there! It may be i'll need loadList externally somewhere down the line but it that happens i can restore this setup
Adds initial list load to list context. Sort/query/pagination logic not added yet.
ItemTable
toTable
ItemGrid
, removes grid display code from ListPane