Adjusted font size in trainings constructions #14
Conversation
There was a problem hiding this comment.
Hey there, great job.
MAIN-Review:
Alle Anforderungen aus dem Issue sind erfüllt (⇒ code implementiert Issue [#3 ])
Code compiles und die Menüs funktionieren.
Auch mehrere Menüs gleichzeitig offen scheinen gut zu funktionieren.
Nitpicks
Fügt vielleicht noch ein paar extra Kommentare ein, um die Lesbarkeit noch weiter zu steigern.
Außerdem setzt bitte die vorgeschlagenen Änderungen noch in die Tat um,
siehe Review-Kommentare.
File can not yet be merged to main!
There was a problem hiding this comment.
Hi, echt gute Arbeit, die gewünschten Punkte sind implementert und funktionieren fast einwandfrei (s. Kommentar). Ich nehme mal an, dass ihr nicht jedes Leerzeichen selbst gesetzt habt hinter den Hastags der Kommentar, aber solche Veränderungen machen die eigentlichen Änderungen etwas unübersichtlich (Vielleicht wurde das irgendwie automatisch gemacht?).
Wenn ihr Fragen habt was genau für ein Problem mit dem automatischen Anpassen vorliegt kontaktiert mich gerne.
Zudem würde ich die default Fenstergröße etwas erhöhen und eine minimale Größe festlegen.
Ansonsten würde ich mich den Punkten von Henry anschließen, vor allem die default Fontsize zu erhöhen.
cellpose/gui/guiparts.py
Outdated
| """ | ||
|
|
||
| from tkinter import * | ||
| import tkinter as tk | ||
| from tkinter import ttk |
There was a problem hiding this comment.
Werden diese tkinter imports genutzt?
cellpose/gui/guiparts.py
Outdated
| # Get the current font size from the combo box | ||
| font_size = int(self.font_size_combo.currentText()) | ||
| # Calculate the new font size based on window height | ||
| new_font_size = max(5, int(self.height() / 30)) |
There was a problem hiding this comment.
Das ist eine gute Umsetzung, echt stark.
Beim skalieren der Schriftgröße sollte allerdings auch die Breite des Fensters berücksichtigt werden, da es sonst bei einem schmalen aber hohen Fenster und großer Schriftgröße selbts adjustziert (und das gerade zu explosionsartig). Ich habe ein bisschen rumprobiert und vermute, dass QT beim resizen immer versucht den gesamten Text des Labels darzustellen und deshalb das Fenter nach unten erweitert.
Um das zu verhindern, könnte man eventuell die Berechnung der new_font_size anpassen und auch von der Breite des Fensters abhängig machen, da sonst bei großer Schrift die Größe als angemessen erachtet wird obwohl das Fenster zu schmal ist. Andere Idee wäre sonst, da Fenster in einem festen Verhältnis nur anpassbar zu machen (z.B. 16:9), aber der erste Weg wäre auf jedenfall optimaler.
Danke für euren Review. Die Leerzeichen habe ich auch erst nach dem pushen bemerkt. Sollen die Leerzeichen Rückgängig gemacht werden oder soll es diesmal so bleiben? |
Gerne kurz rückgängig machen, dann gibts beim mergen da keine Probleme. |
cellpose/gui/guiparts.py
Outdated
| # Calculate the new font size based on window height | ||
| new_font_size = max(5, int(self.height() / 30)) | ||
| # Calculate the new font size based on window height and width | ||
| new_font_size = max(5, max(int(self.height() / 50), int(self.width() / 60))) |
There was a problem hiding this comment.
Das funktioniert leider immer noch nicht, das zuvor beschriebene Problem besteht weiterhin. Ich schicke euch mal ein Video auf Discord um euch zu zeigen was ich meine.
There was a problem hiding this comment.
Hey there, great job.
MAIN-Review:
Alle Anforderungen aus dem Issue sind erfüllt (⇒ code implementiert Issue [#3 ])
Code compiles und die Menüs funktionieren.
Auch mehrere Menüs gleichzeitig offen scheinen gut zu funktionieren.
Nitpicks
Fügt vielleicht noch ein paar extra Kommentare ein, um die Lesbarkeit noch weiter zu steigern.
File can be merged to main!
There was a problem hiding this comment.
Main Review
Das implentierte Feature erfüllt alle gewünschten Bedingungen aus Issue #3.
Die Startschriftgröße und Fenstergröße wurden auf einen sinnvollen Wert angepasst und der Text passt sich dynamisch mit der Fenstergröße an. Ebenso funktioniert die Schriftgrößenauswahl durch den User selbst.
Requested Changes
Alle geforderten Veränderungen wurden implementiert, die Fenstergröße lässt sich nun ohne Probleme anpassen und die Startgröße ist gut gewählt.
Branch can be merged!
Ich weiß nicht genau wie das passiert ist, aber euer Branch wurde auf einem Commit erstellt, der die Änderungen des anderen Scrum Teams bei den Training Prametern bereits enthalten hat. Leider wurden die bei euch wieder verworfen, das wird vermutlich zu einem Konflikt führen, schaut also, dass diese Änderungen erhalten bleiben. (Gemeint ist die Parameter Explanation beim drüberhovern).
Gute Arbeit!
There was a problem hiding this comment.
Feature Text des anderen Features (#5) wurde wieder hinzugefügt.
Funktionalität besteht weiterhin.
Der Branch kann auf Main gemerged werden
Resolves #3
Adjusted font size
The following changes have been made in
TrainHelpWindowclass:layout is now directly assigned to the dialog window:
self.setLayout(layout)label has been made an instance attribute self.label:
self.label = QLabel(text)self.label.setWordWrap(True)layout.addWidget(self.label, 0, 0, 1, 1)created dropdown menu to adjust the font size
added the
adjust_font_sizemethod so that users can adjust font sizeresizeEventhas been overridden to call adjustfont sizewhen the window is resized, ensuring that the window size adjusts when the window size changes