Review Request 110960: FramestackWidget: Make keyboard navigation useful

[Vlas Puhov]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110960/#review35716
-----------------------------------------------------------


You see, the functionality of those two signals intersect, so frameSelectionChanged slot will be called twice if you change current frame by clicking. To solve it you could use selectionChanged signal located in m_frames->selectionModel(), that includes both currentChanged and clicked cases or you could implement missing signal in subclass of QTreeView. 
   If there is something I understand wrong, please let me know.

- Vlas Puhov


On July 8, 2013, 11:04 a.m., Nicolai Hähnle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110960/
> -----------------------------------------------------------
> 
> (Updated July 8, 2013, 11:04 a.m.)
> 
> 
> Review request for KDevelop and Niko Sams.
> 
> 
> Description
> -------
> 
> Allow users to navigate the Frame Stack Toolview using the arrow keys
> and tab/shift-tab without distraction to locate the thread(s) and
> frame(s) of interest.
> 
> This required the following changes:
> 
> 1) Add IDocumentController::DoNotFocus; assorted changes in
> DocumentController and MainWindow.
> 
> The intention is clear: Allow opening and showing a document without
> yanking the focus away from where it currently is. This could also be
> interesting for other toolviews (e.g., keyboard navigation in the Code
> Browser).
> 
> Focusing the new view is hidden inside MainWindow::activateView, so
> I added an overloaded variant of this method which allows the caller
> to choose whether the view should get the focus.
> 
> 2) Use DoNotFocus in the relevant parts of the debugger.
> 
> 3) Change FramestackWidget signal wiring to react to all types of input
> 
> Previously, FramestackWidget connected its slots to the item views'
> clicked() signal, which only reacts on mouse input. Use the selection
> models' currentChanged() signal instead, which reacts to other types of
> input, in particular the keyboard.
> 
> As a consequence, the connections have to be set up in
> currentSessionChanged(), because that's where the model is fully set up.
> 
> Relatedly, switch only m_frames->rootIndex in currentThreadChanged()
> rather than the entire model, because the entire model doesn't actually
> change.
> 
> 4) There is one unrelated change, which is that I moved a connection
> attempt to checkFetchMoreFrames down into currentSessionChanged;
> otherwise, the connection does not succeed (and QObject complains
> about it on the console).
> 
> I noticed that there is a related bug report, bug #316873, about this
> fetch-more-frames feature; the bug report ends rather vaguely without
> resolution, and I only noticed it after starting this Review Request.
> However, fetching more frames automatically, triggered by scrolling,
> works for me.
> 
> That's pretty much it. I hope you consider this a useful new feature,
> and I hope the patch looks good to you. If you see any problems, or if
> I have misstepped outside the coding guidelines at some point, please
> let me know!
> 
> 
> This addresses bug 316873.
>     http://bugs.kde.org/show_bug.cgi?id=316873
> 
> 
> Diffs
> -----
> 
>   debugger/framestack/framestackwidget.h 9794353 
>   debugger/framestack/framestackwidget.cpp dcc7a64 
>   interfaces/idocumentcontroller.h 434cb4d 
>   shell/debugcontroller.cpp 01432e6 
>   shell/documentcontroller.cpp 660637f 
>   sublime/mainwindow.h 421ee57 
>   sublime/mainwindow.cpp dce4bf7 
> 
> Diff: http://git.reviewboard.kde.org/r/110960/diff/
> 
> 
> Testing
> -------
> 
> Only manual testing - I don't know if/how this could be tested
> automatically:
> 
> Within {my own project, the threads test case from kdevelop,
> kdevelop itself} I have interrupted the debug run and done a lot
> of mouse- and keyboard-based back and forth between threads and
> frames. I also explicitly checked that loading additional frames
> still works. That's pretty much it.
> 
> 
> Thanks,
> 
> Nicolai Hähnle
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20130708/81ced192/attachment.html>