Fetch and show operational learning summaries #1307
Conversation
🦋 Changeset detectedLatest commit: 9f26a26 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
roshni73
force-pushed
the
feature/fetch-summaries-key-insights
branch
from
c0f456a
to
2583eac
Compare
roshni73
force-pushed
the
feature/fetch-summaries-key-insights
branch
from
2583eac
to
5a43feb
Compare
roshni73
force-pushed
the
feature/fetch-summaries-key-insights
branch
from
5a43feb
to
8742bd2
Compare
roshni73
force-pushed
the
feature/fetch-summaries-key-insights
branch
from
048ffd3
to
9a4fb10
Compare
samshara
force-pushed
the
feature/fetch-summaries-key-insights
branch
from
202b18d
to
b34d2e8
Compare
samshara
force-pushed
the
feature/fetch-summaries-key-insights
branch
from
3006453
to
e5257de
Compare
| function useSecondarySector(props?: ListProps): [Array<SecondarySector> | undefined, boolean] | ||
| function useSecondarySector(props: PropsForId): [SecondarySector | undefined, boolean] | ||
| function useSecondarySector( | ||
| props?: ListProps | PropsForId, | ||
| ): [SecondarySector | undefined | Array<SecondarySector> | undefined, boolean] { |
There was a problem hiding this comment.
I am not sure if this is the correct implementation for fetching enums/options cc: @frozenhelium @samshara
There was a problem hiding this comment.
@tnagorra should we add these to the DomainContext ?
There was a problem hiding this comment.
When using hooks, users usually expect that this will only make a single request. But, that won't be the case.
We should add this to the DomainContext and make sure the response is cached.
There was a problem hiding this comment.
Fetched and stored secondary sectors in DomainContext.
There was a problem hiding this comment.
I am not sure if this is the correct implementation for fetching enums/options cc: @frozenhelium @samshara
There was a problem hiding this comment.
When using hooks, users usually expect that this will only make a single request. But, that won't be the case.
We should add this to the DomainContext and make sure the response is cached.
There was a problem hiding this comment.
Fetched and stored per components in DomainContext.
| const handleStartDateSelect = useCallback(( | ||
| newValue: string | undefined, | ||
| key: 'appealStartDateAfter', | ||
| ) => { | ||
| onChange( | ||
| newValue, | ||
| key, | ||
| newValue, | ||
| ); | ||
| }, [onChange]); | ||
|
|
||
| const handleEndDateSelect = useCallback(( | ||
| newValue: string | undefined, | ||
| key: 'appealStartDateBefore', | ||
| ) => { | ||
| onChange( | ||
| newValue, | ||
| key, | ||
| newValue, | ||
| ); | ||
| }, [onChange]); | ||
|
|
||
| const handleSearch = useCallback(( | ||
| newValue: string | undefined, | ||
| key: 'appealSearchText', | ||
| ) => { | ||
| onChange( | ||
| newValue, | ||
| key, | ||
| ); | ||
| }, [onChange]); | ||
|
|
There was a problem hiding this comment.
Why not directly use onChange on the components?
There was a problem hiding this comment.
@tnagorra we need to show the labels of the selection as pills component.
There was a problem hiding this comment.
The function signature for handleSearch and onChange are the same so we should be able to just use onChange instead
There was a problem hiding this comment.
@tnagorra here a typing issue arises.
app/src/views/OperationalLearning/Sources/AllExtractsModal/index.tsx
Outdated
Show resolved
Hide resolved
Addresses:
Changes
This PR doesn't introduce:
console.logmeant for debugging