Repository navigation
Split home file - #26
Conversation
| def refresh_departments(): | ||
| university = university_combo.currentText() | ||
| department_combo.clear() | ||
| if university in UNIVERSITIES: | ||
| department_combo.addItems(UNIVERSITIES[university]) | ||
| department_combo.setCurrentIndex(-1) | ||
|
|
||
| university_combo.currentIndexChanged.connect(lambda _: refresh_departments()) |
There was a problem hiding this comment.
Can you explain to me why is there a need to refresh the departments here after detecting a configuration ?
| department = department_combo.currentText() | ||
| if not university or not department: | ||
| status.setText(UI["academic_error_incomplete"]) | ||
| status.setStyleSheet(status.styleSheet().replace("#a6adc8", "#f38ba8")) |
There was a problem hiding this comment.
Why is there styling here ?
| university=university, department=department | ||
| ) | ||
| ) | ||
| status.setStyleSheet(status.styleSheet().replace("#f38ba8", "#a6adc8")) |
| for person in CREDITS: | ||
| frame = QFrame() | ||
| frame.setFrameShape(QFrame.Shape.StyledPanel) | ||
| frame.setStyleSheet(load_qss("pages/card_frame.qss")) |
There was a problem hiding this comment.
Maybe we should not use so many qss files, it kinda hurts the I/O
| name = qlabel(person["name"], size=13, color="#cdd6f4", bold=True) | ||
| name.setStyleSheet( | ||
| name.styleSheet() + " background: transparent; border: none;" | ||
| ) | ||
| fl.addWidget(name) | ||
|
|
||
| role = qlabel(person["role"], size=11, color="#a6adc8") | ||
| role.setStyleSheet( | ||
| role.styleSheet() + " background: transparent; border: none;" | ||
| ) | ||
| fl.addWidget(role) | ||
|
|
||
| if person.get("projects"): | ||
| proj = qlabel( | ||
| UI["projects_prefix"] + ", ".join(person["projects"]), | ||
| size=11, | ||
| color="#8b5897", | ||
| ) | ||
| proj.setStyleSheet( | ||
| proj.styleSheet() + " background: transparent; border: none;" | ||
| ) | ||
| fl.addWidget(proj) |
There was a problem hiding this comment.
Lots of qss that should be moved
| from PyQt6.QtCore import Qt | ||
| from PyQt6.QtWidgets import ( | ||
| QCheckBox, | ||
| QHBoxLayout, | ||
| QMainWindow, | ||
| QPushButton, | ||
| QStackedWidget, | ||
| QVBoxLayout, | ||
| QWidget, | ||
| ) |
There was a problem hiding this comment.
This should trigger ruff, check this with static analysis. Probably unused imports.
| for link in LINKS: | ||
| frame = QFrame() | ||
| frame.setFrameShape(QFrame.Shape.StyledPanel) | ||
| frame.setStyleSheet(load_qss("pages/card_frame.qss")) |
|
|
||
| btn = QPushButton(UI["open_link_button"]) | ||
| btn.setCursor(Qt.CursorShape.PointingHandCursor) | ||
| btn.setStyleSheet(load_qss("pages/link_button.qss")) |
| data = PAGES[key] | ||
| widget, cl = scroll_page(on_back, key) | ||
|
|
||
| body = qlabel(data["body"], size=12, color="#a6adc8", wrap=True) |
There was a problem hiding this comment.
Styling that belongs in qss
| self._autostart_cb.setStyleSheet( | ||
| """ | ||
| QCheckBox { | ||
| color: #a6adc8; | ||
| font-size: 12px; | ||
| background: transparent; | ||
| spacing: 8px; | ||
| } | ||
| QCheckBox::indicator { | ||
| width: 16px; | ||
| height: 16px; | ||
| border: 1px solid #585b70; | ||
| border-radius: 4px; | ||
| background: transparent; | ||
| } | ||
| QCheckBox::indicator:hover { | ||
| border-color: #a6adc8; | ||
| } | ||
| QCheckBox::indicator:checked { | ||
| background-color: #8b5897; | ||
| border-color: #cba6f7; | ||
| image: url(%s); | ||
| } | ||
| QCheckBox::indicator:checked:hover { | ||
| background-color: #9b68a7; | ||
| } | ||
| """ | ||
| % _check_path |
There was a problem hiding this comment.
Yeah, this should be removed also and be placed in styling
TolisSth
left a comment
TolisSth
left a comment
There was a problem hiding this comment.
Overall a solid start but let's continue the refactoring. Start with the comments above and continue with reducing the number of qss files, we should have one or two, not that many files, remember the components are resusable.
|
Δεν έχω ξαναφάει τέτοιο ξύλο ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀ |
What changedCI
Fewer QSS files and reusable components The 16
Because components share rules, all card frames use one Styling that was still in Python
One visible changeThe nav buttons were never styled as intended. This is the only visible change in the PR. It is its own commit, so it can be reverted alone if you prefer the old look. How I checked itAfter every step I compared screenshots before and after, pixel by pixel, offscreen, at 800x600 and 580x500:
Every step gave 0 differences except the nav fix. There, 22 of the 436 images changed, and every changed pixel was inside the 8 nav button rectangles. I also ran the smoke test ( PylintThe new pylint step fails on this branch (5.45/10). I checked I haven't changed any code for pylint, because I'd like your direction first. Would you like docstrings added here, a pylint config change (for example |
| /* Η σειρά έχει σημασία: ο κανόνας "descendant" του scroll (με *) πρέπει | ||
| να μένει πρώτος. Έχει ίδια ειδικότητα με τους κανόνες ρόλων, και | ||
| αν μπει μετά, κερδίζει αυτός και βάφει λάθος παιδιά (hero, | ||
| divider, footer, scroll content). Ο κανόνας "card" πρέπει να μένει | ||
| τελευταίος, μετά τον descendant κανόνα: το card frame είναι μέσα σε | ||
| scroll γονέα, οπότε το * το πιάνει με ίδια ειδικότητα. */ |
There was a problem hiding this comment.
Comments should be written in english
2 participants
Summary
This PR splits
src/unidesk/home.py(555 lines) intohelpers/,ui/, andstyles/, following the structure suggested in the linked issue.home.pyno longer exists; its contents now live across 15 focused modules.Closes #25
Why
home.pymixed data loading, filesystem logic, widget construction, page building, the main window, and ~20 inline QSS strings in one file. This made it hard to navigate and review. The maintainer asked for the split to happen incrementally, with architectural decisions discussed in the PR rather than decided unilaterally. This PR follows that approach: 7 small, independently reviewable commits, each verified before the next began.What changed
helpers/(data and system-interaction logic)text_data.py,academic_config.pymoved as-is from the package root.autostart.py: extracted the autostart toggle logic (AUTOSTART_PATH,is_autostart_disabled(),set_autostart()) that was previously inlined inhome.py.ui/(widget construction)widgets.py: shared helpers (qlabel,divider,back_bar,scroll_page,footer,NavButton), made public (dropped the leading underscore).ui/pages/: one module per page builder (text_page.py,credits_page.py,links_page.py,academic_config_page.py), each importing only what it needs.ui/main_window.py: theUniOSWelcomeclass, moved as a 1:1 relocation (not further decomposed into smaller methods, to keep the one commit that also deleteshome.pyas low-risk as possible).styles/(QSS extraction)setStyleSheet(...)call acrossui/was moved into its own.qssfile understyles/{widgets,main_window,pages}/, loaded viastyles/loader.py(load_qss(filename), usingimportlib.resourcesand@cache, consistent with the existingtext_data.load()pattern)..qssfile per literal, not three files for the whole package. Most of the original strings are selector-less declarations (e.g."background: #110d1a;"). Concatenating these into a handful of files and handing the whole file to every widget would have caused Qt stylesheet parse failures and cross-widget bleeding (the lastbackground:declaration wins, parent stylesheets cascade to children). This was caught and fixed before any code was written, see Verification for how the resulting split was validated against the original behavior.setup.pypackage_dataupdated ("unidesk.styles": ["*/*.qss"]) and confirmed by building a wheel locally and inspecting its contents. All 16.qssfiles are packaged correctly.Bug fixes (small, isolated)
cfg_btn = QPushButton(...)line (the first instance was constructed and immediately discarded).align_rightparameter fromNavButton.__init__(accepted at both call sites, used nowhere in the class body).Docs
README.md's project structure diagram updated to reflecthelpers/,ui/(withpages/), andstyles/.Verification
Every commit was checked before the next began:
ruff check src/unidesk/: same 3 pre-existing errors throughout (SIM115,UP031in the tempfile code,I001inmain.py), zero new ones introduced across the whole series.QT_QPA_PLATFORM=offscreen, constructingUniOSWelcomeand checking page count) after every commit.HEAD, wrote them to.qssfiles, and then asserted byte-identity between what's loaded at runtime and the original literal, for all 20 extracted strings. stdout/stderr from the smoke test was also diffed against aHEADcheckout to confirm no new Qt stylesheet parse warnings were introduced.python -m build --wheelrun locally;unzip -lon the resulting wheel confirms all.qssfiles are present underunidesk/styles/.~/.config/autostart/unidesk.desktop, and the academic config page still saves to~/.unios/academicConfig.json.