Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filemanager1 scroll to item #2280

Merged
merged 27 commits into from
Oct 2, 2024
Merged

Filemanager1 scroll to item #2280

merged 27 commits into from
Oct 2, 2024

Conversation

jeremypw
Copy link
Collaborator

@jeremypw jeremypw commented Aug 9, 2023

Fixes #2214

In order to make the behaviour that expected and the same regardless of whether Files is already running or not when the request is made it was necessary to fix several problems, in particular some races occurring when Files was starting up.

  • Fix identifying whether a requested location was a child of a tab
  • Fix AbstractDirectory function selecting files race with ffocus file on startup
  • ViewContainer function focusing a location waits for loading to complete if called at startup

To test:

Call Files from the command line with a non-directory file parameter that is a child of one of the tabs that is restored on startup, on that contains a lot of files and requires scrolling to view the requested child.

Do similar where requested tab is not restored.

Do similar when Files is already running and is/is not showing the requested tab.

Use "Open in Folder" function of browser or other app with similar function.

In each case Files should show the requested item selected and visible.

Check for regressions where the requested item is a folder. The requested folder should open in a new tab if it is not already open, even if it is a child of an existing tab.

@danirabbit danirabbit requested a review from a team August 14, 2023 21:03
@jeremypw jeremypw marked this pull request as draft September 28, 2023 10:20
@jeremypw
Copy link
Collaborator Author

Needs further testing following change to async adding tabs.

@jeremypw jeremypw marked this pull request as ready for review September 28, 2023 11:53
@jeremypw
Copy link
Collaborator Author

jeremypw commented Sep 28, 2023

There is some uncertainty about when Files should create duplicate a tab when the requested folder is already showing (has been restored). At the moment duplicates are created on startup but not if Files is already running.

@zeebok
Copy link
Contributor

zeebok commented Apr 12, 2024

The code itself looks fine to me. I can try to fit in time to test this over the weekend, but if you feel you tested enough already I don't mind approving

@zeebok
Copy link
Contributor

zeebok commented Apr 19, 2024

I am testing on eOS7:

Call Files from the command line with a non-directory file parameter that is a child of one of the tabs that is restored on startup, on that contains a lot of files and requires scrolling to view the requested child.
Do similar where requested tab is not restored.

Both seem to always open a new tab, but it does scroll to the correct file with it already selected. Is it creating the tab before restoring or something?

Do similar when Files is already running and is/is not showing the requested tab.

This behaves as expected. Creates a tab if needed or switches to an existing one of the correct directory with desired file selected

Use "Open in Folder" function of browser or other app with similar function.

Works as expected I think. Only situation of note is that if Files is closed, Open in will open a new tab even if the correct folder is a restored tab. (ie Downloads/ tab will be restored and a new Downloads/ tab will open from the browser) similar to the issue I saw from terminal.

Check for regressions where the requested item is a folder. The requested folder should open in a new tab if it is not already open, even if it is a child of an existing tab.

This looks to work alright

@jeremypw
Copy link
Collaborator Author

@zeebok Are you able to approve this for merging? You had a look some time ago.

Copy link
Contributor

@zeebok zeebok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing that you never responded to was the behavior of a new tab always being created. If this isn't a concern then go ahead and merge

@jeremypw
Copy link
Collaborator Author

jeremypw commented Oct 2, 2024

@zeebok Ah, I overlooked/forgot that - I havent looked at this for a while. I think it could be annoying if a series of tabs at the same location got opened so I'll fix it so that doesn't happen (if possible).

@jeremypw jeremypw marked this pull request as draft October 2, 2024 12:14
@jeremypw
Copy link
Collaborator Author

jeremypw commented Oct 2, 2024

@zeebok On second thoughts, the issue you mentioned only seems to occur if Files is not already running and the requested folder/file was saved for restoration. If Files is already running when the request is made the duplicate tabs are not created. I think there has been some discussion in the past (maybe in the context of Terminal) about the correct behaviour when a requested folder is amongst those that are going to be restored and I think the current wisdom is that the restored folders should be independent of the requested folder(s). It is certainly easier that way as folders are restored asynchronously.
So I'll merge for now and see if any issues are raised.

@jeremypw jeremypw marked this pull request as ready for review October 2, 2024 15:18
@jeremypw jeremypw merged commit 7a8038d into main Oct 2, 2024
4 checks passed
@jeremypw jeremypw deleted the filemanager1-scroll-to-item branch October 2, 2024 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When opened with specific file to select, the view is not showing that file
3 participants