web reporter refactor and issueFormService - #212951
Conversation
|
Adding myself for reviewing overall structure and layering of the code, not so much deep detail of it. |
Andrea Mah (andreamah)
left a comment
Andrea Mah (andreamah)
left a comment
There was a problem hiding this comment.
Did a quick review- I haven't done much CSS in this repo, so this is just a cursory look. Nice work!
There was a problem hiding this comment.
I only glanced at the layer structure and I like the direction this is going but I see a future in where the issue reporter - even on desktop - is fully operated and opened from the workbench (via window.open). This would help ensuring we are using the same solution on all platforms and probably allows for even more code reuse. I suggest to do that move as a follow up next month 👍
As for where all the issue related files go that are currently in vs/workbench/issue, I would suggest to move them into the existing vs/workbench/contrib/issue folder (into the respective browser and electron-sandbox folders there). The issue reporter is imho a contribution to the workbench and not otherwise used by core services, so I think it qualifies for the contrib layer. Even though its weird to have a contrib being bundled in our build files, this would go away once we remove all electron-main pieces.
Benjamin Pasero (bpasero)
left a comment
Benjamin Pasero (bpasero)
left a comment
There was a problem hiding this comment.
Why do we need to keep some of the files in workbench/electron-sandbox/issues? They belong into the contrib/issue folder as well, no?
Benjamin Pasero (bpasero)
left a comment
Benjamin Pasero (bpasero)
left a comment
There was a problem hiding this comment.
Pretty close now for me at least.
Benjamin Pasero (bpasero)
left a comment
Benjamin Pasero (bpasero)
left a comment
There was a problem hiding this comment.
Good from my end, pending the verification in a real build.
* web version working
* change to mainWindow
* PROPER MOVEMENT
* working for web as well
* move issueFormService to workbench/contrib/issue
* cleaning up{
* more cleanup, added setting
* styling
* use mainwindow to open and closee
* css fixes
* fix css again
* fix CSS and wonky applyCSS rules
* change gulpfile
* add and update system info
* address some of the comments
* move files! small changes
* move JS and non window specific back to electron sandbox
* fix on issueReporter.js
* fix build file
* fix gulp file too....
* move everything into contrib
* fix workbench import
* move everything else into contrib, fix import
* change name to web
* applying more feedback fixes :D
* fix command and remove unused import:
* add back issueTroubleshoot
* fix gulpile outputs
* fix out exclusion:
6 participants
more refactoring and adding in web functionality.