Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Fixes #1945 Restore library panels #2639

Merged
merged 2 commits into from
Feb 11, 2020
Merged

Fixes #1945 Restore library panels #2639

merged 2 commits into from
Feb 11, 2020

Conversation

keianhzo
Copy link
Contributor

Fixes #1945 Restore library panels. I really don't like to have a flag to know if the user has initiated the interaction so if there is a better alternative I'd be happy to change this. Without the flag the panels would be closed during the restore when about:blank and home are loaded as we always close them when there is a navigation event.

@keianhzo keianhzo self-assigned this Jan 14, 2020
@keianhzo keianhzo added this to the #9 polish milestone Jan 14, 2020
Copy link
Contributor

@bluemarvin bluemarvin left a comment

Choose a reason for hiding this comment

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

Getting a first run crash when updating FxR with this patch applied.

@keianhzo keianhzo requested a review from bluemarvin January 24, 2020 17:45
@keianhzo
Copy link
Contributor Author

keianhzo commented Feb 3, 2020

This should be ready for review.

@bluemarvin
Copy link
Contributor

I agree this isn't pretty but I can't think of anything cleaner. @MortimerGoro last chance to comment before landing it.

@MortimerGoro
Copy link
Contributor

@bluemarvin @keianhzo would it be possible to use a URL based approach instead of adding the extra flags and enum?

For example map about://history or about://bookmarks kind of URLs to the library panels

@bluemarvin
Copy link
Contributor

@bluemarvin @keianhzo would it be possible to use a URL based approach instead of adding the extra flags and enum?

For example map about://history or about://bookmarks kind of URLs to the library panels

That could be workable. I'm not sure how we would make it work with the GV history system though. I like the concept, it isn't clear how we would implement it.

@keianhzo
Copy link
Contributor Author

@bluemarvin @MortimerGoro That a good idea, I've added support for it. It doesn't seem to mess up the history and it's a much cleaner solution for restoring.

@keianhzo keianhzo requested a review from bluemarvin February 10, 2020 11:06
Copy link
Contributor

@bluemarvin bluemarvin left a comment

Choose a reason for hiding this comment

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

Much cleaner. Seems to work well too.

@MortimerGoro MortimerGoro merged commit 5cecc09 into master Feb 11, 2020
@MortimerGoro MortimerGoro deleted the v9/restore_library branch February 11, 2020 18:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Save/restore library panels when the app is reopened
3 participants