Conversation
quang-le
left a comment
quang-le
left a comment
There was a problem hiding this comment.
Just the enum thing that is a bit annoying I know.
The rest is mostly comments at this stage
js/src/constants/legend.ts
Outdated
| export enum LegendTypes { | ||
| THRESHOLD = 'threshold', | ||
| ORDINAL = 'ordinal', | ||
| SCALE = 'scale', | ||
| } |
There was a problem hiding this comment.
Don't use TS enums please, they are TS dirty little secret. On a serious note, when using enums, TS code in not just JS code + types anymore, so I'm not sure the babel plugin we use support it (we don't use the TS compiler IIRC, we just remove all TS typing)
There was a problem hiding this comment.
Allright, I'll replace this with a json object then
| const groupedOptions = useMemo( | ||
| () => | ||
| groupOptions | ||
| ? options.reduce( |
There was a problem hiding this comment.
We're using reduce to produce a dictionary, and then we convert the dictionary to a list to iterate on its values?
I think that:
forEachwould be more legible- You should output an array directly
There was a problem hiding this comment.
Yeah, I do like the syntax of reduce though, downside is indeed the double loop through, let me change that.
| <MenuList> | ||
| {groupOptions ? ( | ||
| Object.values(groupedOptions).map(group => ( | ||
| <Fragment key={group.label}> | ||
| <ListSubheader>{group.label}</ListSubheader> | ||
| <DropdownOptionItems | ||
| options={group.options} | ||
| onClick={handleOnClick} | ||
| /> | ||
| </Fragment> | ||
| )) | ||
| ) : ( | ||
| <DropdownOptionItems | ||
| options={options} | ||
| onClick={handleOnClick} | ||
| /> | ||
| )} | ||
| </MenuList> |
There was a problem hiding this comment.
It's probably worth grouping this and groupedOptions in a component at some point, when the feature get finalized
There was a problem hiding this comment.
I would split that in 2 components
There was a problem hiding this comment.
You mean, in two files ?
| }, | ||
| }; | ||
|
|
||
| const list_field_key = 'matching_criteria'; |
There was a problem hiding this comment.
Uppercase for constants. We're not consistent with this, I know, but I think it's good practice
js/src/domains/planningV2/components/scenarioRule/ScenarioRuleModal.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| export const useCreateUpdateScenarioRule = ( | ||
| scenarioId: number, | ||
| ): UseMutationResult => |
There was a problem hiding this comment.
You can type this when you have the backend
| import { useSnackMutation } from 'Iaso/libs/apiHooks'; | ||
| import { MESSAGES } from '../../messages'; | ||
|
|
||
| export const useDeleteScenarioRule = (scenarioId: number): UseMutationResult => |
| // ); | ||
|
|
||
| // return response?.results || []; | ||
| return dummyScenarioRulesResponse; |
There was a problem hiding this comment.
There's a waitFor util that allows yo to make the queryFn truly async so you can test the loading state
Bewi
force-pushed
the
SNT-285-i-can-create-a-assignment-rule
branch
from
6c3c337 to
05a40b7
Compare
2 participants
What problem is this PR solving?
Initiate V2 of intervention assignment, purpose of this is only to have the form, BE is not ready yet.
Related JIRA tickets
SNT-285
Changes
What is done:
What still need to be done:
How to test
Print screen / video
N/A
Notes
This is a first PR, state is far from final but to reduce the size of the PR I will do this in multiple steps.
This is not visible by the end user unless they know the url, in which case, it is fine.
This does not yet connect to the Backend
Related IASO PR: BLSQ/iaso#2752
Doc
N/A