-
-
Notifications
You must be signed in to change notification settings - Fork 105
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
Add segmented control style #6
Add segmented control style #6
Conversation
Something I'd like to have feedback on: using Auto Layout to pin the content views to the window size; because on the one hand, it makes window size management trivial, but on the other you end up with somewhat unexpected constraint animations when the window size change is animated. |
@sindresorhus Did you have a chance to take a look? You want me to adjust something? |
I really appreciate the PR. I’ve just been too busy in real life to look at it yet (wedding and family visits). I’ll hopefully have a chance to look at it the coming week :) |
The latest commit adds a representation for what we may call "overflow menu", which is how AppKit displays segmented controls in toolbars that outgrow the window. Example with a few duplicates of the 1st example app's preference to make the control too big: In addition, we should also prevent this from happening by keeping a minimum window size, I would say. But nevertheless, if users force the window size down, this is a better fallback than the previous behavior, i.e. displaying an empty menu. |
@sindresorhus Did you have a chance to assess the code so far? :) |
Today's changes make prefs work with Auto Layout properly.
|
This is looking really promising. I finally have a chance to look through the code now. |
One thing I noticed is that when using the segmented control style and I change a tab, it should animate from the segmented buttons, meaning, the segmented buttons should not move. |
I think it could be useful to have a public method to change tabs while the preferences window is open. Something like: preferencesWindowController.change(to: .preferenceAdvanced) |
Example/AppDelegate.swift
Outdated
|
||
func applicationWillFinishLaunching(_ notification: Notification) { | ||
window.orderOut(self) | ||
} | ||
|
||
func applicationDidFinishLaunching(_ notification: Notification) { | ||
preferencesWindowController.showWindow() | ||
preferencesWindowController.showWindow(toolbarItemIdentifier: .preferenceAdvanced) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would read better as:
preferencesWindowController.showWindow(withPreference: .advanced)
Or something similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since showWindow
would do the same as change(to:)
, what do you think about the latest change to showPreference
? I hope it brings across that it opens the window if needed, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's good.
@DivineDominion Hey, still interested in finishing this? You are so close :) |
I can have a look at this with one of my next app's iterations. I find it surprisingly hard to animate both the window frame and the content size smoothly, so growing the window symmetrically left and right to make the segmented controls stick could take more time than I can spare in the upcoming week or two. I'll schedule time for this, though. Thanks for the review so far :) Good to know it's only minor things. |
Smaller icons would work, but look odd, so I think we should leave them disabled completely for now
Cache all tab sizes first when the views are new and use these sizes. Segmented controls at the top seem to affect the window frame size.
Now that it's not implicitly assumed anymore that all preferenceables _are_ indeed view controllers, but could also be configuration objects, the name should reflect this change.
Using NSTabBarViewController makes switching between the controls hard: segmented controls are not supposed to be part of the toolbar.
Gotcha. Changed it. Also fixed the remaining issues. Centering the window is jiggling the segmented control around a bit, probably due to rounding errors. I tried |
From the HIG:
We don't currently do that. I think we should do it when users call |
@sindresorhus Will happily add that, but again favor doing that in another PR to keep this focused on the segmented controls. Would that work with you? |
Yup, agreed. This PR is already very large. Moved it to a new issue: #16 |
Can you update the readme to reflect the changes? |
I don't really know a solution to that either. If you cannot find a solution, I'm fine with just deferring it to later and move it to a new issue. |
I think I caught everything in the README. I also added screenshots of the two styles, but oddly the README preview on my branch doesn't work. I hope it's just due to it being a fork and relative paths not working correctly, but can you double-check this when you merge? |
Didn’t we decide on |
Otherwise, the changes look great! 👌 |
You're right, this totally went under my radar. I added a docstring for now; when #16 is implemented, the recommendation can be expanded and will make more sense IMHO |
This is absolutely amazing. I'm very happy about the end result. Thank you so much for working so hard on this, @DivineDominion. I really appreciate it. 🙌 Should we do a new release with what we have here now or do you intend to work on more things in the near future? Also, would you be interested in joining the project as a maintainer? No commitment required. Just want to highlight your contribution in the readme. |
I forgot to double-check the images before merge, but they are definitely corrupt. Would you mind submitting them again? |
I was transitioning all my macOS apps to this framework, so I have sufficient skin in the game to help you with this. If you can use a helping hand, I'd be happy to contribute! I don't have a strong opinion about releases. In fact, I prefer more frequent releases over big releases that take a long time. Adding this is kind of a big thing, compared to the original code base, so why not! Then fix the open issues and localization problems. |
When you have done so, I would be happy to list your apps in the readme. |
I also replied to #6 (comment), in case it gets lost. |
Cool! Am currently using this for: Am transitioning this one when we're done with the next release: |
Can you do a PR to add them to the readme? Maybe a new section above the FAQ section. |
Closes #2
I struggled a while with different approaches until I threw out
NSTabViewController
out of the window and began managing the toolbar in a custom controller.Now you can configure the style upon initialization and have it your way. Replicating the transition animation was a bit tricky because the user could click faster than the transition, ending in an invalid view state; I added a
PausableWindow
that swallows user events to prevent this from happening without disabling the toolbar controls.Enhancements I'd prefer to tackle in separate issues/PRs:
isCentered
/alignment
,isSizedUniformly
(all segments have the same with; that's the current state)isCentered
/alignment
,toolbarStyle
(icon, label, icon and label)PreferencesWindowController
: makePreferencesWindowController
internal, exposePreferencesController
as a plain Swift object with a simple APIinit(fromCoder:)
)Changes to the PR: