Skip to content
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

Clone from Run UI is terrible with JSON #2714

Closed
martin-laurent opened this issue Dec 10, 2019 · 8 comments · Fixed by #2747
Closed

Clone from Run UI is terrible with JSON #2714

martin-laurent opened this issue Dec 10, 2019 · 8 comments · Fixed by #2747
Assignees
Labels

Comments

@martin-laurent
Copy link

What happened:
Try to clone a run (via the clone run button) that uses JSON strings as run parameters.
While the JSON run parameters show decently enough on the run's config tab (see #2713 for issues about that),
cloning such a run shows minified JSON in the form. This is a terrible user experience for someone that just wants to iterate on hyperparameters for example.

What did you expect to happen:
A similar but editable snippet of JSON to be rendered instead of the minified line of text

What steps did you take:
Cloned a run that contains JSON like run parameters

Attached is a screenshot of the config page that renders JSON snippet:
Screen Shot 2019-12-10 at 3 31 26 PM

and its direct equivalent when clicking on the clone run button:
Screen Shot 2019-12-10 at 3 31 58 PM

@Bobgy
Copy link
Contributor

Bobgy commented Dec 13, 2019

/area frontend
/assign @Bobgy
/kind feature

@Bobgy
Copy link
Contributor

Bobgy commented Dec 13, 2019

Thanks! That sounds useful.

I don't currently have time to work on this, anyone is welcome to contribute before I've got some bandwidth on this.
/priority p2

@Bobgy
Copy link
Contributor

Bobgy commented Dec 13, 2019

Providing a button nearby that can change the text box to a multiline editor might be helpful.

@Bobgy
Copy link
Contributor

Bobgy commented Dec 13, 2019

/priority p1
bumping a little priority because this is low effort and a solid use case.

@fenglixa
Copy link
Member

I would like to provide PR for this issue
BR
-Feng

@Bobgy
Copy link
Contributor

Bobgy commented Dec 23, 2019

Thanks @fenglixa! You are welcome, but @drewbutlerbb4 has already made a PR. Let's avoid duplicate efforts.

@fenglixa
Copy link
Member

Yes, I found that already PR on this issue. Thanks @Bobgy @drewbutlerbb4

@Bobgy
Copy link
Contributor

Bobgy commented Jan 10, 2020

Removing the label, as it is obviously more complex than I thought. Thanks @drewbutlerbb4! Well thought through the use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants