Fix unmodeled methods always being marked as unsaved#2969
Conversation
When opening a library group in the model editor, unmodeled methods would always be marked as unsaved, even if there were no changes. This was because the `ModelKindDropdown` component did not properly take into account that the `kind` for an unmodeled method should be an empty string. It would always try setting it to `undefined`, which would cause the method to be marked as unsaved. This fixes it by checking if there are valid kinds before setting the kind to the first one.
|
|
||
| if (kinds.length === 0) { | ||
| if (value !== "") { | ||
| onChangeKind(""); |
There was a problem hiding this comment.
I've tested it and this PR works. It took me a little while to understand why.
What confused me is that in onChangeKind it aborts and does nothing if modeledMethod === undefined, so in the case of there being no modeled method, this PR shouldn't be a change in behaviour. And in the case where there is a modeled method but type === "none", then the according the typing, kind should never be undefined.
But the issue isn't with value being undefined to start with, and instead it was in value !== undefined && !kinds.includes(value) where the kinds array would be empty so we'd call onChangeKind(undefined).
This PR fixes things by adding a check for kinds.length === 0 earlier.
Looking into this made me realise that we no longer call these inputs with modeledMethod: undefined in the model editor panel. In MethodRow we always construct a dummy modeled method with type: "none".
| @@ -63,7 +63,16 @@ export const ModelKindDropdown = ({ | |||
|
|
|||
| useEffect(() => { | |||
| const value = modeledMethod?.kind; | |||
There was a problem hiding this comment.
I find the logic in this method a little confusing because of multiple if statements in sequence where some return early and some don't. I think we could also perhaps make value never be undefined and that could simplify the logic.
What do you think of:
const value = modeledMethod?.kind ?? "";
if (kinds.length === 0 && value !== "") {
onChangeKind("");
} else if (kinds.length > 0 && !kinds.includes(value)) {
onChangeKind(kinds[0]);
}2 participants
When opening a library group in the model editor, unmodeled methods would always be marked as unsaved, even if there were no changes. This was because the
ModelKindDropdowncomponent did not properly take into account that thekindfor an unmodeled method should be an empty string. It would always try setting it toundefined, which would cause the method to be marked as unsaved. This fixes it by checking if there are valid kinds before setting the kind to the first one.Checklist
ready-for-doc-reviewlabel there.