Differential D11703

Select and scroll to the file for "Open path" and "Open Path in New Window" operations
ClosedPublic
Actions

Authored by ngraham on Mar 26 2018, 12:20 AM.

Details

Reviewers
elvisangelaccio
Group Reviewers
Dolphin
Commits
R318:9536d1170a9f: Select and scroll to the file for "Open path" and "Open Path in New Window"…
Summary

After performing a search and using the "Open path" or "Open Path in New Window" present in the context menu, make sure the file is selected and visible in the resulting view.

Cannot implement the same fix for "Open Path in New Tab" because of a limitation in how the contents of inactive tabs are rendered; will need to fix that separately in another patch.

BUG: 377510

Test Plan

Search for an item, and choose Open Path or Open path in new window

Diff Detail

Repository
R318 Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham requested review of this revision.Mar 26 2018, 12:20 AM
ngraham created this revision.
ngraham added a comment.Mar 26 2018, 12:23 AM

Discussion points:

  • Should the new tab functions be in a separate patch?
  • Open path in new tab successfully selects the file in the new tab, but m_tabWidget->nextTabPage()->activeViewContainer()->view()->markUrlAsCurrent(item.url()); scrolls to an incorrect location. This does not happen with the Open Path, and I'm not sure what accounts for the difference. I could use a hand figuring out what's happening here.
ngraham added a comment.Mar 30 2018, 1:57 PM

Friendly ping!

elvisangelaccio added a subscriber: elvisangelaccio.Mar 31 2018, 9:52 AM

Can you explain how to reproduce the "scrolls to an incorrect location" issue?

(I'm having an hard time trying to test this patch because the baloosearch ioslave crashes a lot here...)

ngraham added a comment.Mar 31 2018, 4:57 PM
In D11703#237544, @elvisangelaccio wrote:

Can you explain how to reproduce the "scrolls to an incorrect location" issue?

  • Apply patch and launch Dolphin
  • Open Find panel
  • Search for anything
  • Right-click on any entry and select Open path in new tab

Works just fine for Open and Open in new window though.

elvisangelaccio added a comment.Apr 2 2018, 8:54 PM

Ok, I was finally able to reproduce the issue. I'll investigate in the next days...

elvisangelaccio added a comment.Apr 7 2018, 3:36 PM
In D11703#238754, @elvisangelaccio wrote:

Ok, I was finally able to reproduce the issue. I'll investigate in the next days...

Ok, it doesn't work because in KItemListView::scrollToItem() we get an empty geometry() (since the new tab has not been activated yet).

I don't see an easy way to fix it, the view engine clearly had not been designed with this use case in mind :/

elvisangelaccio requested changes to this revision.Apr 7 2018, 3:40 PM
elvisangelaccio added inline comments.
src/dolphinmainwindow.cpp
834–835

Please remove, since this approach doesn't work.

src/dolphintabwidget.h
46–52 ↗(On Diff #30574)

Let's add these two functions in another patch, since we don't need them here.

This revision now requires changes to proceed.Apr 7 2018, 3:40 PM
ngraham updated this revision to Diff 31647.Apr 8 2018, 4:21 AM

Undo new tab changing methods that belong in another patch, and the scroll-to-and-select-file feature for the new tab case, since it won't work with other changes elsewhere first

ngraham marked an inline comment as done.Apr 8 2018, 4:22 AM
ngraham added a comment.Apr 8 2018, 4:57 AM

Moved the new tab access functions to D12039: Add more tab access functions.

elvisangelaccio added a comment.Apr 8 2018, 8:55 AM

Commit message also needs to be updated (it should not mention 'open in new tab' or it should say why we are ignoring it).

ngraham retitled this revision from Select and scroll to the file for "Open path" operations to Select and scroll to the file for "Open path" and "Open Path in New Window" operations.Apr 8 2018, 3:21 PM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
elvisangelaccio accepted this revision.Apr 9 2018, 8:23 PM
This revision is now accepted and ready to land.Apr 9 2018, 8:23 PM
Closed by commit R318:9536d1170a9f: Select and scroll to the file for "Open path" and "Open Path in New Window"… (authored by ngraham). · Explain WhyApr 9 2018, 11:06 PM
This revision was automatically updated to reflect the committed changes.
ngraham mentioned this in R318:b12a7b0f24d5: Add more tab access functions.

Revision Contents
Changeset List

PathPackages
Msrc/dolphinmainwindow.cpp (4 lines)

Diff 31781

View Options

src/dolphinmainwindow.cpp

Loading...