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

Feature: Added an option to keep Files running in the background for quicker startup time #13236

Merged
merged 15 commits into from
Aug 24, 2023

Conversation

hishitetsu
Copy link
Member

@hishitetsu hishitetsu commented Aug 23, 2023

Resolved / Related Issues

  • Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
    Closes Feature: Cache windows for quicker startup #12614

Validation
How did you test these changes?

  • Did you build the app and test your changes?
  • Did you check for accessibility? You can use Accessibility Insights for this.
  • Did you remove any strings from the en-us resource file?
    • Did you search the solution to see if the string is still being used?
  • Did you implement any design changes to an existing feature?
    • Was this change approved?
  • Are there any other steps that were used to validate these changes?
    1. Turn on "Leave app running in the background when the window is closed" in Advanced settings
    2. Close and restart the app.
    3. The app will launch quicker.

Known Issues

  • When only one tab is opened, the tab is not visually selected.
    image
  • Closing the main window while a property window is still open freezes the property window until the next time the main window is opened.
  • The content flashes while it loads.

@yaira2 yaira2 self-requested a review August 23, 2023 04:01
@hishitetsu hishitetsu marked this pull request as ready for review August 23, 2023 13:16
@hishitetsu
Copy link
Member Author

All known issues were resolved. Now ready for review.

@Lukiluc29
Copy link
Contributor

Lukiluc29 commented Aug 23, 2023

Hello, I just have one last little thing to ask. Is it normal that when I close "Files" and then reopen it, the status center appears? Because before, whether I checked "Open a new tab" or "Continue where you left off," it wouldn't appear when I had performed an action before closing it.
Capture d'écran 2023-08-23 154237

@hishitetsu
Copy link
Member Author

Hello, I just have one last little thing to ask. Is it normal that when I close "Files" and then reopen it, the status center appears? Because before, whether I checked "Open a new tab" or "Continue where you left off," it wouldn't appear when I had performed an action before closing it.

It is true that the behavior is different from when the process is terminated without caching, but I think it doesn't matter.

@yaira2 yaira2 changed the title Feature: Added support for caching the main window for quicker startup Feature: Added an option to keep Files running in the background for quicker startup time Aug 23, 2023
@Lukiluc29
Copy link
Contributor

It is true that the behavior is different from when the process is terminated without caching, but I think it doesn't matter.

Yes, but it's a bit bothersome because when I close Files, I don't necessarily want to see the status center when I reopen Files, especially when I haven't taken any action upon its reopening.

@yaira2
Copy link
Member

yaira2 commented Aug 23, 2023

@Lukiluc29 this is the expected behavior because you aren't actually closing Files, you're leaving it open in the background.

@Lukiluc29
Copy link
Contributor

@Lukiluc29 this is the expected behavior because you aren't actually closing Files, you're leaving it open in the background.

Users tend to close applications after using them, and they might find it disruptive to encounter a status center that they didn't initiate themselves. By removing the status center in this scenario, we can enhance the user's sense of control and provide a smoother and more intuitive interaction with the application.

@hishitetsu
Copy link
Member Author

I rather think that notifications should remain even after restarting the app unless the user explicitly clears them. In any case, this is something we can discuss as a separate issue.

src/Files.App/App.xaml.cs Outdated Show resolved Hide resolved
@Lukiluc29
Copy link
Contributor

I rather think that notifications should remain even after restarting the app unless the user explicitly clears them. In any case, this is something we can discuss as a separate issue.

Where can we discuss this?

@yaira2 yaira2 requested a review from d2dyno1 August 23, 2023 21:30
@hishitetsu
Copy link
Member Author

I rather think that notifications should remain even after restarting the app unless the user explicitly clears them. In any case, this is something we can discuss as a separate issue.

Where can we discuss this?

You can open a new issue once this PR is merged.

@hishitetsu hishitetsu requested a review from yaira2 August 24, 2023 02:25
@mdtauk
Copy link
Contributor

mdtauk commented Aug 24, 2023

When the app is in the background, could Files use the system notifications to alert on the completion of on-going tasks?

@hishitetsu
Copy link
Member Author

When the app is in the background, could Files use the system notifications to alert on the completion of on-going tasks?

It stops the thread, so that may be difficult as it is now.
A related issue is already open. #8207

@yaira2
Copy link
Member

yaira2 commented Aug 24, 2023

For what it's worth, file operations already continue in the background even if the app is fully terminated. The missing part is displaying a notification, and that's being tracked in the issue @hishitetsu linked.

Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

Looks great to me in terms of the codebase quality!

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Aug 24, 2023
Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

This is a really nice improvement, it's a lot more enjoyable to use Files when you don't have to wait for it to load. LGTM!

@yaira2 yaira2 merged commit 2659136 into files-community:main Aug 24, 2023
@hishitetsu hishitetsu deleted the CacheMainWindow branch August 24, 2023 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Cache windows for quicker startup
5 participants