-
Notifications
You must be signed in to change notification settings - Fork 4
add field description to modification #3546
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
| const rootNetworks = useSelector((state: AppState) => state.rootNetworks); | ||
| const isMonoRootStudy = useSelector((state: AppState) => state.isMonoRootStudy); | ||
| const highlightedModificationUuid = useSelector((state: AppState) => state.highlightedModificationUuid); | ||
| const [hoveredRowIndex, setHoveredRowIndex] = useState<number>(); |
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.
no initial value for this state ?
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.
maybe -1 ?
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.
and no need to precise type if there is a default value
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 when I precised -1 as a default value, the hovered index was lost when the page rerendered. It is surprising because according to react help it shouldn't. I don't have the explanation.
src/components/graph/menus/network-modifications/DescriptionRenderer.tsx
Outdated
Show resolved
Hide resolved
src/components/graph/menus/network-modifications/DescriptionRenderer.tsx
Outdated
Show resolved
Hide resolved
| import { setModificationDescription } from '../../../../services/study/network-modifications'; | ||
|
|
||
| export interface DescriptionRendererProps extends ICellRendererParams<NetworkModificationMetadata> { | ||
| setModifications: React.Dispatch<SetStateAction<NetworkModificationMetadata[]>>; |
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.
Don't import the whole react dependency, but just dispatch like it's done for set state action.
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 you made me realise that I don't need that prop anymore. Removed : 1dcc6d6
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.
Okay, but you still import React
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.
sorry : removed here : adbd106
src/components/graph/menus/network-modifications/DescriptionRenderer.tsx
Outdated
Show resolved
Hide resolved
src/components/graph/menus/network-modifications/DescriptionRenderer.tsx
Outdated
Show resolved
Hide resolved
src/components/graph/menus/network-modifications/network-modifications-table.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
| */ | ||
| import { ICellRendererParams } from 'ag-grid-community'; | ||
| import { DescriptionModificationDialog, EditNoteIcon, NetworkModificationMetadata } from '@gridsuite/commons-ui'; | ||
| import React, { useCallback, useState } from 'react'; |
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.
You still have a global react import
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.
sorry : removed here : adbd106
| import { setModificationDescription } from '../../../../services/study/network-modifications'; | ||
|
|
||
| export interface DescriptionRendererProps extends ICellRendererParams<NetworkModificationMetadata> { | ||
| setModifications: React.Dispatch<SetStateAction<NetworkModificationMetadata[]>>; |
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.
Okay, but you still import React
src/components/graph/menus/network-modifications/DescriptionRenderer.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
|



PR Summary
allows to add a description to individual netmods in the tree view using : gridsuite/commons-ui#955
moves NetworkModificationMetadata to commons-ui