-
-
Notifications
You must be signed in to change notification settings - Fork 403
doc: Add QT MVC structure to style guide #950
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
base: main
Are you sure you want to change the base?
doc: Add QT MVC structure to style guide #950
Conversation
class structure guide inspired by: https://www.pythonguis.com/tutorials/pyside6-modelview-architecture/ |
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.
Overall looking very good so far, just a couple of early nits and comments
STYLE.md
Outdated
- The Controller is just called `MyCoolWidget` instead of `MyCoolWidgetController` as it will be directly used by other code | ||
- the ui elements are in private variables | ||
- this enforces that the controller shouldn't directly access UI elements | ||
- instead the view should provide a protected API (e.g. `_get_color()`) for things like setting/getting the value of a dropdown, etc. |
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.
Might be also worth giving properties a shoutout here
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.
done
STYLE.md
Outdated
- the ui elements are in private variables | ||
- this enforces that the controller shouldn't directly access UI elements | ||
- instead the view should provide a protected API (e.g. `_get_color()`) for things like setting/getting the value of a dropdown, etc. | ||
- the callback methods are already defined as protected methods with NotImplementedErrors | ||
- defines the interface the callbacks | ||
- enforces that UI events be handled |
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.
Small general nit for capitalization
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.
should be better now
I have now added everything I wanted to and thus this is ready for final review |
Summary
Splits the style guide into a separate file, and documents the intended future structure of the qt part of the code base.
Tasks Completed
Not applicable