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

Restore active preference when reopening window #26

Merged
merged 4 commits into from
Apr 20, 2019
Merged

Restore active preference when reopening window #26

merged 4 commits into from
Apr 20, 2019

Conversation

DivineDominion
Copy link
Collaborator

Closes #16

@SamusAranX
Copy link
Contributor

Safari keeps track of the last opened preference pane even after complete app restarts. Based on a cursory glance, I don't think this does.

@sindresorhus
Copy link
Owner

We need to check what the Human Interface Guidelines says about that and what other apps do. I would prefer to not have that behavior, personally.

@DivineDominion
Copy link
Collaborator Author

Well:

Restore the last viewed preference pane. If the user switches preference panes, your app should remember this change and show the same pane immediately the next time the user opens your preferences window.
https://developer.apple.com/design/human-interface-guidelines/macos/app-architecture/preferences/

That could mean either.

Restoring state is very broad, too, and says "Preserve and restore your app’s state so people can continue where they left off" and "Configure reopened windows as expected".

But what's expected in terms of preferences? I would not expect the app to restore my last preferences editing session, because I don't do that a lot.

But if an app relies on preferences modifications heavily + requiring restarts, this can get on one's nerves. Does this happen often? At all?

Wouldn't be hard to use UserDefaults to restore the last session, but is that really a use case?

@DivineDominion
Copy link
Collaborator Author

DivineDominion commented Apr 17, 2019

Merged current master into this to make it ready for merging.

Since the HIG aren't clear, I suggest the following:

  1. we start with the interpretation that is least invasive/requires the least amount of code change, i.e. restore the session while the app is running (= this PR)
  2. we discuss opening the lib for the other interpretation, e.g. adding an option to store the last open tab in the user defaults, and handle that separately

/// - Parameter preferencePane: Identifier of the preference pane to display.
/// - Note: Unless you need to open a specific pane, prefer not to pass a parameter at all or `nil`.
/// - Parameter preferencePane: Identifier of the preference pane to display, or `nil` to show the
/// tab that was open when the user last closed the window.
public func show(preferencePane preferenceIdentifier: PreferencePaneIdentifier? = nil) {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
public func show(preferencePane preferenceIdentifier: PreferencePaneIdentifier? = nil) {
public func show(preferencePane preferencePaneIdentifier: PreferencePaneIdentifier? = nil) {

?

I think we should be consistent in using preferencePane over just preference.

@sindresorhus
Copy link
Owner

we start with the interpretation that is least invasive/requires the least amount of code change, i.e. restore the session while the app is running (= this PR)

👍

we discuss opening the lib for the other interpretation, e.g. adding an option to store the last open tab in the user defaults, and handle that separately

I don't think we should do this at all. Most of Apple's apps don't do it, neither does most third-party apps. Instead, I think we should open a Radar to get Apple to clarify in the HIG exactly what's expected with restoring. What do you think?

@DivineDominion
Copy link
Collaborator Author

Found a bug and fixed it :) I'm fine with your verdict @sindresorhus as we can still move on merging this PR and then never talk about restoring the tab after relaunching the app. Works for me :)

@sindresorhus sindresorhus merged commit 2bb3fc7 into sindresorhus:master Apr 20, 2019
dezinezync pushed a commit to dezinezync/Preferences that referenced this pull request Dec 17, 2022
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.

Remember the last active tab and show it by default
3 participants