-
Notifications
You must be signed in to change notification settings - Fork 11
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
[Platform] Use React for state management in literature section #416
base: ot-literature-section
Are you sure you want to change the base?
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.
Please remove old highlighted folder/files of the PR to reflect the concise changes
Is it best to leave them until we are sure no other tweaks for the state mgmt are needed? - all profile pages show only the new literature widget then the old for easy comparison. (Admittedly, it would have been better to leave the original folder name unchanged!) |
No. We have tools as git, GitHub, GitHub spaces and our dev environments to compare and/or revert. It is important for better PRs reviews to be clear about the intended code added/removed |
OK, should be cleaner now. |
// type LiteratureListParameter = { | ||
// id: string, | ||
// name: string, | ||
// entity: any, // ADD LATER | ||
// BODY_QUERY: string, | ||
// definition: any, // ADD LATER | ||
// }; |
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.
Remove commented
const DetailsDispatchContext = createContext(null as any); | ||
|
||
function literatureReducer(literatureState: LiteratureStateType, action: any) { | ||
console.log(`LITERATURE REDUCER: ${action.type}`); |
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.
Remove log
loadingEntities: false, | ||
}; | ||
|
||
const LiteratureContext = createContext(null as any); |
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 use the LiteratureStateType
const LiteratureContex = createContext<LiteratureStateType>(initial_state);
}; | ||
|
||
const LiteratureContext = createContext(null as any); | ||
const LiteratureDispatchContext = createContext(null as any); |
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.
For typing the actions, you can use the same pattern of AotF FocusState
ot-ui-apps/apps/platform/src/components/AssociationsToolkit/context/AssociationsFocusContext.tsx
Line 18 in 4e9fc2e
export enum FocusActionType { |
} | ||
|
||
function detailsReducer(detailsState: DetailsStateType, action: any) { | ||
console.log(`DETAILS REDUCER: ${action.type}`); |
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.
remove log
export function useLiterature() { | ||
return useContext(LiteratureContext); | ||
} | ||
|
||
export function useLiteratureDispatch() { | ||
return useContext(LiteratureDispatchContext); | ||
} | ||
|
||
export function useSelectedCategories() { | ||
const { category } = useLiterature(); | ||
return [...category].sort(); | ||
} | ||
|
||
export function useDisplayedPublications() { | ||
const { page, pageSize, litsIds } = useLiterature(); | ||
return isEmpty(litsIds) ? [] : getPage(litsIds, page, pageSize); | ||
} | ||
|
||
export function useDetails() { | ||
return useContext(DetailsContext); | ||
} | ||
|
||
export function useDetailsDispatch() { | ||
return useContext(DetailsDispatchContext); |
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.
👍 Like this approach. cc: @chinmehta
<Fade in> | ||
<Box mb={2}> | ||
<Skeleton height={60} /> | ||
{/* <Box pt="1px"> */} |
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.
remove comment
Description
Use React instead of Recoil for state management in literature section.
Issue: #3292
Type of change
How Has This Been Tested?
Checked in dev.
Checklist: