feat: Always Show Workflow Parameters #7809
Conversation
kennytrytek
force-pushed
the
kgt/always-submit-workflow-params
branch
from
65d30cf
to
2799bc3
Compare
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Hi Stale bot, I'm planning on updating this today. |
kennytrytek
force-pushed
the
kgt/always-submit-workflow-params
branch
from
2799bc3
to
6293e12
Compare
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
kennytrytek
force-pushed
the
kgt/always-submit-workflow-params
branch
from
6293e12
to
29ebabd
Compare
kennytrytek
force-pushed
the
kgt/always-submit-workflow-params
branch
2 times, most recently
from
5bda1c3
to
2b2bb7a
Compare
| value: p.name === parameter.name ? event.value : this.getValue(p), | ||
| enum: p.enum | ||
| })); | ||
| this.setState(update); |
There was a problem hiding this comment.
do you want to use this.setState(oldState => newState) version here?
There was a problem hiding this comment.
There is only one example of the pattern you suggested in the code base:
https://github.com/argoproj/argo-workflows/blob/v3.3.1/ui/src/app/shared/components/input-filter.tsx#L63
What would this code gain by using that pattern? All other setStates in the UI look like this.
There was a problem hiding this comment.
it is useful it you only want to modify part of the state, which is what I think you are trying to do here
There was a problem hiding this comment.
I think I'd do this differently, I'd add a new parameters to displaySelectFieldForEnumValues named onChange and pass in the correct func to it. The decouples the two functions.
There was a problem hiding this comment.
it is useful it you only want to modify part of the state
I'm not convinced that setState(update) does not do a partial update. At least, I haven't been able to find any documentation that reads otherwise. I also tried using the arrow function update anyway, but was unable to make it successfully update the state.
I'd add a new parameters to displaySelectFieldForEnumValues named onChange and pass in the correct func to it
I understand the sentiment here, but I gave up after a few hours of trying to make it work this way. I ran into two problems:
- The update function would lose reference to
this. - The
setStateupdate would appear to work, but didn't actually update any values.
If you have any more specific direction, or just want someone else to make the changes, that would be fine with me.
| entrypoint: workflowEntrypoint, | ||
| entrypoints: this.props.templates.map(t => t.name), | ||
| selectedTemplate: defaultTemplate, | ||
| parameters: this.props.workflowParameters || [], | ||
| parameters: [] as Parameter[], | ||
| workflowParameters: this.props.workflowParameters, |
There was a problem hiding this comment.
do we need to make a copy of this object rather than reference if we're going to change it? Use Object.assign to copy an object
There was a problem hiding this comment.
Changed to a copy in 44da063.
| value: p.name === parameter.name ? event.value : this.getValue(p), | ||
| enum: p.enum | ||
| })); | ||
| this.setState(update); |
There was a problem hiding this comment.
I think I'd do this differently, I'd add a new parameters to displaySelectFieldForEnumValues named onChange and pass in the correct func to it. The decouples the two functions.
| })) | ||
| }); | ||
| const update = {} as State; | ||
| update[parameterStateName] = this.state[parameterStateName].map(p => ({ |
- Fixes argoproj#7311 Signed-off-by: Kenny Trytek <kenneth.g.trytek@gmail.com>
- Fixes argoproj#7311 Signed-off-by: Kenny Trytek <kenneth.g.trytek@gmail.com>
kennytrytek
force-pushed
the
kgt/always-submit-workflow-params
branch
from
2b2bb7a
to
44da063
Compare
Signed-off-by: Kenny Trytek kenneth.g.trytek@gmail.com
Fixes #7311
Testing
The following GIF shows the submission of the example workflow from #7311 (comment).