344 refactor UI logic #347
Conversation
Mathieu25900
left a comment
Mathieu25900
left a comment
There was a problem hiding this comment.
Nice job improving the UI code by modifying the current and temporary file path of main.py. This simplifies the logic, makes it simpler to maintain and fix. When the global state is updated, it makes the logic easier to debug.
Jahnavidarisetti
left a comment
Jahnavidarisetti
left a comment
There was a problem hiding this comment.
You’ve greatly enhanced the clarity and separation of concerns in the project. The main entry point is now much cleaner and easier to follow, with all event handlers and UI construction logic moved out to purpose specific files.
Testing across UI platforms is required.
| @@ -0,0 +1,26 @@ | |||
| """ | |||
There was a problem hiding this comment.
Good job extracting this logic into a dedicated module. This improves separation of concerns and makes the main entry point cleaner. Consider adding or expanding unit tests for these newly isolated functions if feasible.
There was a problem hiding this comment.
Certainly, once we have the RFID scanners I'll be able to test them fully.
There was a problem hiding this comment.
@amalladi017 Just to clarify, what @Jahnavidarisetti is referring to is automated unit testing, not manual testing with hardware. We can run automated unit tests quickly and frequently without hardware attached. This may require creating additional software components to enable this, such as a mock, a virtual version of the hardware that provides fake inputs useful for automated testing.
Check out these resource for extra details:
| ) | ||
|
|
||
|
|
||
| def build_menu(root, experiments_frame, rfid_serial_port_controller): |
There was a problem hiding this comment.
The docstring provides clear context on the purpose and usage of the function. Please ensure that parameter and return value types are also specified where relevant, especially as the codebase grows.
| @@ -0,0 +1,115 @@ | |||
| """ | |||
There was a problem hiding this comment.
Great work centralizing all UI command callbacks in this module! Organizing the event handlers here makes the codebase much more maintainable and easier to follow. The clear docstrings and consistent structure add to the readability this is a solid improvement to the project.
|
When I ran main, there was no issue. The program worked for me |
|
@Jahnavidarisetti I noticed there are some new commits, but it's still unmerged. Were all your requirements met? |
Fixes #344
What was changed?
The main application UI logic was refactored out of main.py into dedicated modules under the ui/ folder. Files created include root_window.py, menu_bar.py, welcome_screen.py, and commands.py, each handling a distinct part of the interface.
Why was it changed?
The previous main.py was too large and mixed UI layout with application logic. This change improves modularity, readability, and maintainability, and prepares the codebase for future extensions or testing.
How was it changed?
I followed PEP8 style conventions, enforced with Pylint 3.0, and resolved all violations related to import order, naming, trailing whitespace, and missing docstrings. Where needed, I suppressed warnings only with justification.