Skip to content
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

Lomakkeen tuottaminen templaatista #38

Merged
merged 5 commits into from
Oct 30, 2024

Conversation

nmaarnio
Copy link
Collaborator

No description provided.

@nmaarnio nmaarnio linked an issue Oct 25, 2024 that may be closed by this pull request
@nmaarnio nmaarnio requested a review from LKajan October 25, 2024 11:12
@nmaarnio nmaarnio force-pushed the 14-lomakkeen-tuottaminen-templaatin-konfiguraatiosta branch from 4b60927 to 2349588 Compare October 30, 2024 07:45
Copy link
Contributor

@LKajan LKajan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!
Let's go with this one and modify later on. I added few minor comments.

We should probably at some point change more towards a mvc pattern and create/fill a model from the template and create the form from the model.

Comment on lines +22 to +34
class PlanRegulationGroupWidget(QWidget, FormClass): # type: ignore
"""A widget representation of a plan regulation group."""

delete_signal = pyqtSignal(QWidget)

def __init__(self, feature: Feature):
super().__init__()
self.setupUi(self)

# TYPES
self.frame: QFrame
self.heading: QLineEdit
self.del_btn: QPushButton
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that its a common practice to define the types in the class section as following:

Suggested change
class PlanRegulationGroupWidget(QWidget, FormClass): # type: ignore
"""A widget representation of a plan regulation group."""
delete_signal = pyqtSignal(QWidget)
def __init__(self, feature: Feature):
super().__init__()
self.setupUi(self)
# TYPES
self.frame: QFrame
self.heading: QLineEdit
self.del_btn: QPushButton
class PlanRegulationGroupWidget(QWidget, FormClass): # type: ignore
"""A widget representation of a plan regulation group."""
# TYPES
frame: QFrame
heading: QLineEdit
del_btn: QPushButton
delete_signal = pyqtSignal(QWidget)
def __init__(self, feature: Feature):
super().__init__()
self.setupUi(self)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used to do it like this too, until at some point VSCode stopped recognizing this sort of typing for me... If it is an editor bug, I will look into it more


# TYPES
self.regulation_kind: QLineEdit
self.form_layout: QFormLayout = self.layout()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think layouts are accessible by the layout name defined in QtDesigner so no need the function call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it depends whether the layout is added as a separate item or only set to a widget unless I am mistaken. The top-level layout in this widget was only "set" and not as a separate entry in the widget tree.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "main" layouts of widgets are not shown in the widget tree but the name of the used layout is configurable in the widget's property editor under the layout section.

@LKajan LKajan force-pushed the 14-lomakkeen-tuottaminen-templaatin-konfiguraatiosta branch from e92f907 to fda2581 Compare October 30, 2024 11:07
@LKajan LKajan merged commit 483f193 into main Oct 30, 2024
5 checks passed
@LKajan LKajan added the enhancement New feature or request label Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lomakkeen tuottaminen templaatin konfiguraatiosta
2 participants