Gui goto #9177
Gui goto #9177
Conversation
Signed-off-by: LucasYuki <[email protected]>
Signed-off-by: LucasYuki <[email protected]>
LucasYuki
requested review from
gadfort and
maliberty
and removed request for
gadfort
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several valuable improvements to the GUI's "Go to position" functionality. Separating the 'Update' and 'Goto' actions is a great enhancement for user experience. The refactoring of the zoom padding calculation in LayoutViewer::zoomTo and the addition of LayoutViewer::getVisibleDiameter to invert this operation are well-conceived. However, I've identified a logical issue in the implementation of getVisibleDiameter related to how scrollbar dimensions are handled, which will lead to incorrect calculations. My review includes a detailed comment and a suggested fix for this issue.
Signed-off-by: LucasYuki <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: LucasYuki <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
I'm holding of reviewing until you make the changes to the update button we discussed. |
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: LucasYuki <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
If I switch to 'Show DBU' it is should use integers (shows microns & dbus for the same values) |
|
An oddity is that if I enter Size=5 and hit goto I get a slightly different value. I am not hitting the edges of the area |
Unfortunately, there are integer conversions that introduce errors from the requested size (float) to the view (pixels), so the inverse function is a little bit inaccurate. |
Signed-off-by: LucasYuki <[email protected]>
The GUI already does the conversion automatically. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| &LayoutTabs::viewUpdated, | ||
| goto_dialog_, | ||
| &GotoLocationDialog::updateUnits); | ||
| &GotoLocationDialog::updateLocation); |
There was a problem hiding this comment.
warning: no header providing "gui::GotoLocationDialog" is directly included [misc-include-cleaner]
src/gui/src/mainWindow.cpp:20:
- #if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
+ #include "src/gotoDialog.h"
+ #if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
|
Please fix the tidy |
Signed-off-by: LucasYuki <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
2 participants
Closes #8700
Changes: