-
Notifications
You must be signed in to change notification settings - Fork 11
162 repair validation #208
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: develop
Are you sure you want to change the base?
162 repair validation #208
Conversation
w1stler
left a comment
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.
- add commit with styling the code
.../widget/containers/main_body/content/application/application_tabs/applicants_tab/__init__.py
Outdated
Show resolved
Hide resolved
Rafalkufel
left a comment
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.
I think this part about validation are redundant. All others: good job 💪
| # When showing the application container as part of UI flow we | ||
| # don't want to immediately highlight pristine fields. Use the | ||
| # internal _validate with show_errors=False to just check validity | ||
| # without triggering visual validation. Full validation (with | ||
| # highlights) will be triggered when user submits or switches tabs. | ||
| if hasattr(self.application_container, "_validate"): | ||
| return self.application_container._validate(show_errors=False) | ||
| # fallback |
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.
Main task of this method is validate + highlight application, and all it's tabs. It should be called before we print documents. I don't think that adding another responsibility to it (ie show_errors=False) is correct.
| # Backwards-compatible validate method. | ||
| # By default this behaves as before and will show field-level | ||
| # validation results. If called with show_errors=False it will | ||
| # only check `is_valid` flags (no display/highlighting) which is | ||
| # useful when toggling visibility of the application form so we | ||
| # don't highlight pristine fields immediately. | ||
| return self._validate(show_errors=True) | ||
|
|
||
| def _validate(self, show_errors: bool = True) -> bool: | ||
| """Internal validation helper. |
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.
Generally we have:
- method
validateto mainly highlight errors of this component, and child components and additionally returns boolean of if given component is valid - property
is_validto only check if given component (and child components) is valid, without highlighting errors.
Your change tries to push this two into validate. Do we really need it?
| self.application_container = parent | ||
| layout = QVBoxLayout(self) | ||
| layout.setAlignment(Qt.AlignTop) | ||
| layout.setAlignment(Qt.AlignmentFlag.AlignTop) |
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.
👍
84131c7 to
b403d61
Compare
Solve #162 issue.
Fix margins for headers and sections in "Create Application" tabs
Adjusted spacing to make headers and sections more aligned and readable.
Fix warnings overflowing tabs in the top navbar
Increased tab width so the ⚠ icon for invalid validation fits properly and does not overflow.
Fix initial validation in tabs
Form fields are no longer marked as validated when first entering the tab.
Fix sidebar menu button hover after resetting forms
After resetting forms, sidebar buttons now properly respond to hover.