-
Notifications
You must be signed in to change notification settings - Fork 59
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
fix: show field types on json launch form #199
Conversation
226191d
to
8399e6d
Compare
Codecov Report
@@ Coverage Diff @@
## master #199 +/- ##
=========================================
Coverage ? 65.97%
=========================================
Files ? 354
Lines ? 7407
Branches ? 1256
=========================================
Hits ? 4887
Misses ? 2520
Partials ? 0 Continue to review full report at Codecov.
|
|
||
if (keys.includes('title')) { | ||
const { title, type, format } = data; | ||
data['title'] = `${title} (${format ? format : type})`; |
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.
what about to use ${format ?? type}
instead?
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.
That’s a good catch, David. I just forgot to replace it with '??' as I wrote separate .js file to write this function. When I used in .js file, it was throwing an error saying SyntaxError: Unexpected token '?'
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.
Will push a fix ASAP.
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.
I think the error is thrown coz the js file was not using ES6 or later versions.
For ts, it's okay. :)
if (keys.includes('title')) { | ||
const { title, type, format } = data; | ||
data['title'] = `${title} (${format ? format : type})`; | ||
if (!keys.includes('additionalProperties')) return data; |
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.
What if we have other fields but don't have additionalProperties
?
Just to make sure what this means on my end.
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.
'additionalProperties' is all I can see from the api response for now. If there is anything more, we can cope with it later.
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.
Yeah, we can do that later.
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.
Could you let me know your ideas?
Signed-off-by: Pianist038801 <[email protected]>
8399e6d
to
d7368f2
Compare
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.
LGTM!
## [0.25.1](http://github.com/lyft/flyteconsole/compare/v0.25.0...v0.25.1) (2021-09-13) ### Bug Fixes * show field types on json launch form ([#199](http://github.com/lyft/flyteconsole/issues/199)) ([a42b9f8](http://github.com/lyft/flyteconsole/commit/a42b9f8520fcd24dee752111e606ad9ae9bd88f5)) * this is rfc, do not deploy until resolved; details in comments ([#172](http://github.com/lyft/flyteconsole/issues/172)) ([67dd183](http://github.com/lyft/flyteconsole/commit/67dd18397caf40e350da40e0672e500eaa9f338a))
🎉 This PR is included in version 0.25.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Signed-off-by: Pianist038801 <[email protected]> Co-authored-by: Pianist038801 <[email protected]> Signed-off-by: Jason Porter <[email protected]>
## [0.25.1](http://github.com/lyft/flyteconsole/compare/v0.25.0...v0.25.1) (2021-09-13) ### Bug Fixes * show field types on json launch form ([#199](http://github.com/lyft/flyteconsole/issues/199)) ([a42b9f8](http://github.com/lyft/flyteconsole/commit/a42b9f8520fcd24dee752111e606ad9ae9bd88f5)) * this is rfc, do not deploy until resolved; details in comments ([#172](http://github.com/lyft/flyteconsole/issues/172)) ([67dd183](http://github.com/lyft/flyteconsole/commit/67dd18397caf40e350da40e0672e500eaa9f338a)) Signed-off-by: Jason Porter <[email protected]>
Signed-off-by: Pianist038801 <[email protected]> Co-authored-by: Pianist038801 <[email protected]> Signed-off-by: Jason Porter <[email protected]>
## [0.25.1](http://github.com/lyft/flyteconsole/compare/v0.25.0...v0.25.1) (2021-09-13) ### Bug Fixes * show field types on json launch form ([#199](http://github.com/lyft/flyteconsole/issues/199)) ([a42b9f8](http://github.com/lyft/flyteconsole/commit/a42b9f8520fcd24dee752111e606ad9ae9bd88f5)) * this is rfc, do not deploy until resolved; details in comments ([#172](http://github.com/lyft/flyteconsole/issues/172)) ([67dd183](http://github.com/lyft/flyteconsole/commit/67dd18397caf40e350da40e0672e500eaa9f338a)) Signed-off-by: Jason Porter <[email protected]>
* feat: add workflow versions table Signed-off-by: csirius <[email protected]> Signed-off-by: Jason Porter <[email protected]> * chore(release): Release 0.25.0 [skip ci] # [0.25.0](http://github.com/lyft/flyteconsole/compare/v0.24.0...v0.25.0) (2021-08-31) ### Features * add workflow versions table ([#193](http://github.com/lyft/flyteconsole/issues/193)) ([6fff87e](http://github.com/lyft/flyteconsole/commit/6fff87e40007fd15faae634eb6402045c067dd2c)) Signed-off-by: Jason Porter <[email protected]> * improvement: show proper error message for aborted workflows (#195) * improvement: show proper error message for aborted workflows Signed-off-by: Pianist038801 <[email protected]> * improvement: show abort message in the execution list Signed-off-by: Pianist038801 <[email protected]> Co-authored-by: Pianist038801 <[email protected]> Signed-off-by: Jason Porter <[email protected]> * Fix/versions executions gap (#197) * feat: add workflow versions table Signed-off-by: csirius <[email protected]> * fix: space between versions and executions table Signed-off-by: csirius <[email protected]> Signed-off-by: Jason Porter <[email protected]> * feat: workflow version details page Signed-off-by: csirius <[email protected]> Signed-off-by: Jason Porter <[email protected]> * Pre merge checkin Signed-off-by: Jason Porter <[email protected]> * fix: this is rfc, do not deploy until resolved; details in comments (#172) * fix: this is rfc, do not deploy until resolved; details in comments Signed-off-by: Jason Porter <[email protected]> * fix: this is rfc, do not deploy until resolved; details in comments note: deploying with caution; the believe if that it was incorrect to use meta. Signed-off-by: Jason Porter <[email protected]> * fix: show field types on json launch form (#199) Signed-off-by: Pianist038801 <[email protected]> Co-authored-by: Pianist038801 <[email protected]> Signed-off-by: Jason Porter <[email protected]> * chore(release): Release 0.25.1 [skip ci] ## [0.25.1](http://github.com/lyft/flyteconsole/compare/v0.25.0...v0.25.1) (2021-09-13) ### Bug Fixes * show field types on json launch form ([#199](http://github.com/lyft/flyteconsole/issues/199)) ([a42b9f8](http://github.com/lyft/flyteconsole/commit/a42b9f8520fcd24dee752111e606ad9ae9bd88f5)) * this is rfc, do not deploy until resolved; details in comments ([#172](http://github.com/lyft/flyteconsole/issues/172)) ([67dd183](http://github.com/lyft/flyteconsole/commit/67dd18397caf40e350da40e0672e500eaa9f338a)) Signed-off-by: Jason Porter <[email protected]> * Minor fixes Signed-off-by: Jason Porter <[email protected]> * Feat/version details (#198) * feat: add workflow versions table Signed-off-by: csirius <[email protected]> * feat: workflow version details page Signed-off-by: csirius <[email protected]> Signed-off-by: Jason Porter <[email protected]> * fix: repopulate struct input fields on relaunch form (#201) Signed-off-by: Pianist038801 <[email protected]> Co-authored-by: Pianist038801 <[email protected]> Signed-off-by: Jason Porter <[email protected]> * Graph ux feature add legend (#196) * Checkin and merge to master Signed-off-by: Jason Porter <[email protected]> * fixed one more Signed-off-by: Jason Porter <[email protected]> * chore(release): Release 0.25.2 [skip ci] ## [0.25.2](http://github.com/lyft/flyteconsole/compare/v0.25.1...v0.25.2) (2021-09-16) ### Bug Fixes * repopulate struct input fields on relaunch form ([#201](http://github.com/lyft/flyteconsole/issues/201)) ([950e080](http://github.com/lyft/flyteconsole/commit/950e080a2c52c6294630f0ffd1410b0e45b50a8d)) Signed-off-by: Jason Porter <[email protected]> * final before pr Signed-off-by: Jason Porter <[email protected]> * Fixed issues created by bad rebase Signed-off-by: Jason Porter <[email protected]> * Fixed more merge issues Signed-off-by: Jason Porter <[email protected]> Co-authored-by: csirius <[email protected]> Co-authored-by: flyte bot <[email protected]> Co-authored-by: pianist <[email protected]> Co-authored-by: Pianist038801 <[email protected]> Co-authored-by: csirius <[email protected]>
This PR makes JSON Launch Form show field types like other inputs.
Type
Are all requirements met?
Tracking Issue
fixes flyteorg/flyte#1419