#2401: Rework Mainwindow - #2437
samuelkos17 wants to merge 24 commits into
Conversation
Coverage Report for CI Build 35987805879Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.2%) to 74.339%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions24 previously-covered lines in 3 files lost coverage.
Coverage Stats💛 - Coveralls |
laim2003
left a comment
There was a problem hiding this comment.
Thanks for the deep rework! It looks way better already. The architecture is now much cleaner and allows for other features to be built on top. Regarding the TabFactory/general Tab architecture I had some optimization ideas that we should think about. But anyways good work!
Co-authored-by: Lukas <luk.faber@gmx.de>
36f3911 to
61208de
Compare
61208de to
118e69e
Compare
4877144 to
d4ff27a
Compare
1201487 to
e22ce9a
Compare
…amuelkos17/IDEasy into feature/2401-rework-mainwindow
hohwille
left a comment
There was a problem hiding this comment.
@samuelkos17 thanks for your PR.
I very much like your work and can already see that we achieve the lose-coupling and more modular and maintainable code this way. Great work 👍
I focused on two things that we should rather get aligned also now and not when tons of screens (aka dialogs) have been implemented (and need to be refactored):
- packaging layout
- dependency management to access technical components (required almost everywhere like e.g.
NlsServicealongside with our famousIdeContext. Just think to add another such dependency in the future. You will have to add it to many constructors everywhere causing quite some refactoring that we maybe can avoid.
6b37763 to
ea71607
Compare
ea71607 to
bbab107
Compare
hohwille
left a comment
There was a problem hiding this comment.
@samuelkos17 thanks for the update. For me all looks good now and I would consider this as ready to merge. 👍
Maybe we should wait until the release is through but if nobody else has concerns we should then finally merge to open the road for follow-up changes.
|
@hohwille yes we can wait til the release is done. While updating my review for the docu in #2291 I've also realized that I didn't use the guiStateManager god-object everywhere and still had a few useless parameters in constructors so I cleaned that up and pushed it. It's only a small change and IMHO you don't really need to look at it. |
This PR fixes #2401 #2402 #2433
Implemented changes:
This is quite a huge PR that completely reworked our GUI architecture, the most important changes:
The Refactoring-Work isn't done with this PR though, every functionality we had before (besides the language selection) is also present in the new GUI, but the GUI architecture is still not clean enough and there are a few hacks/smells in the code that still need to be reworked. In order to not bloat this PR even further the next tasks are listed here:
Testing instructions
Please add conscise, understandable instructions on how a reviewer can test/verify the functionality of your contribution here:
ide guiusing the local dev build and test if everything works.Checklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.batand notfeature/921 fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)with
internalpom.xmlfiles or otherwise if runtime dependencies changed, you have updated our LICENSE.asciidoc