Pipeline start wizard #335
Conversation
Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
…to pipeline-start-wizard Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
Codecov Report
@@ Coverage Diff @@
## master #335 +/- ##
==========================================
- Coverage 73.87% 72.51% -1.37%
==========================================
Files 46 47 +1
Lines 3415 3500 +85
Branches 635 640 +5
==========================================
+ Hits 2523 2538 +15
- Misses 892 962 +70
Continue to review full report at Codecov.
|
|
The look and feel matches VSCode settings style much closer. However, it feels like we could do something to clean up the multiple duplicate fields |
| @@ -0,0 +1,18 @@ | |||
| { | |||
| "compilerOptions": { | |||
There was a problem hiding this comment.
Why do you need a tsconfig here?
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const path = require('path'); | ||
|
|
||
| module.exports = { |
There was a problem hiding this comment.
Do you need the lower level webpack for building the webview?
If so, can you explain why you can't do it at the root level?
There was a problem hiding this comment.
We have several webviews, which build mostly independant SPA, I'm not sure how to do that with singe webpack config
There was a problem hiding this comment.
We put all the views in specific folder and just generate webpack config. This way you can build all the views running webpack once.
https://github.com/redhat-developer/vscode-openshift-tools/blob/master/src/view/webpack.config.js
| } | ||
| } | ||
|
|
||
| export class NavigationItem extends BaseWidget { |
There was a problem hiding this comment.
This comment is optional. As a rule, I prefer to keep it one class per file. Multiple classes in a file do not effect performance, that is why I say it is optional. This is a general style that is followed but not a requirement.
However, I separate them because it is easier to reason over them, find them, and test them.
|
@joshuawilson That ' multiple duplicate fields' I add only for sample(to record demo) they do not exist on this PR code, sorry for confusion. |
|
And to be clear @sudhirverma continue work on this |
There was a problem hiding this comment.
Should we consider something more advanced than plain html? There are 3k libraries out there that would provide API similar to well known frameworks. One example is preact https://preactjs.com/.
|
Why add complexity? Why ask them to refactor a 3rd time? Is there something wrong with the code? |
|
Closing this PR as implemented in #338 |
This PR provide a very basic implementation of wizard UI using only TS and CSS.
Demo:
Fix: #310