Add fit to cts viewer#4849
Conversation
Signed-off-by: Joanne Low <jlowpc@yahoo.com>
| connect(exit_, &QAction::triggered, this, &MainWindow::exit); | ||
| connect(this, &MainWindow::exit, viewers_, &LayoutTabs::exit); | ||
| connect(fit_, &QAction::triggered, viewers_, &LayoutTabs::fit); | ||
| connect(fit_, &QAction::triggered, clock_viewer_, &ClockWidget::fit); |
There was a problem hiding this comment.
Thanks for looking into this.
Fit in the main window and fit in the clock viewer should be separate. You might well want to fit one without fitting the other. You should create an independent action for the ClockWidget.
There was a problem hiding this comment.
When you say independent action for the ClockWidget, do you want a different hot key for the fit for clock viewer? If so, what key should I use? If not, please let me know how do we decide when user press 'f', whether it goes to the layout window or the clock tree viewer? Thanks.
|
clang-tidy review says "All clean, LGTM! 👍" |
|
How about also having a fit button in the clock widget? And, instead of having two hotkeys couldn't we fit based on the cursor position? (Only fit the clock tree if cursor is inside this widget) |
|
I was thinking the fit key would be window dependent as well. There is already a fit on the right click menu so I'm not sure another button is needed. |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@maliberty please confirm that you want the 'f' key to ONLY work on the CTS window when the mouse cursor is within the CTS window, and you want the 'f' key to ONLY work on the layout when the mouse cursor is over the layout window. The menu and button for fit will work ONLY for the layout regardless of where the mouse cursor is. |
|
My thought was it would depend on the mouse location or focus but perhaps this is getting too tricky both for implementation and somewhat unintuitive as a UI. A fit button next to the update button might be a simpler solution. |
Signed-off-by: Joanne Low <jlowpc@yahoo.com>
|
Please review and comment on my attempt #2 to add a fit button instead of using 'f' hotkey for CTS viewer. Please use attached smaller and improved test case for testing [source run.tcl]. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| void ClockWidget::fit() | ||
| { | ||
| if (views_.size() > 0 && clocks_tab_->currentIndex() < views_.size()) { |
There was a problem hiding this comment.
warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
| if (views_.size() > 0 && clocks_tab_->currentIndex() < views_.size()) { | |
| if (!views_.empty() && clocks_tab_->currentIndex() < views_.size()) { |
Additional context
/usr/include/c++/11/bits/stl_vector.h:1006: method 'vector'::empty() defined here
empty() const _GLIBCXX_NOEXCEPT
^
|
Thanks this look good. Please run clang-format on the code and fix the clang-tidy warning above. |
Signed-off-by: Joanne Low <jlowpc@yahoo.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Thanks! |
3 participants
Attempt to add fit to cts viewer Issue #2590. Attached a simple ibex test case (with 2 clock tabs) to test the hotkey 'f'.
test_cts_viewer_fit.tar.gz