-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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
feat: custom positioning for traffic light buttons #21781
Conversation
💖 Thanks for opening this pull request! 💖 We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix. Examples of commit messages with semantic prefixes:
Things that will help get your PR across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
/cc @electron/wg-api for more inputs. |
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.
In terms of API I'd prefer
trafficLightPosition: {
x: number;
y: number;
}
Then it is just a Point object
shell/browser/native_window_mac.mm
Outdated
@@ -509,6 +511,45 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { | |||
[NSEvent removeMonitor:wheel_event_monitor_]; | |||
} | |||
|
|||
void NativeWindowMac::RepositionTrafficLights() { | |||
if (!traffic_light_offsetX_ && !traffic_light_offsetY_) { |
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.
This does not allow offsets set to 0
which someone may want
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.
My thinking was that an offset of 0 should result in the default position of the traffic lights (in titleBarStyle: 'hidden'
mode). Are you suggesting there's a use case where someone might want an offset smaller than that default position? In that case, negative numbers would work here.
IMO (x: 0, y: 0) should have a frame of reference relative to the default position, and not the absolute (0, 0) position of the frame
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.
Seems legit 👍
Congrats on merging your first pull request! 🎉🎉🎉 |
Release Notes Persisted
|
* feat: custom positioning for traffic light buttons * remove NSLog and unnecessary call-site in IsVisible() * no longer need to check if entering fullscreen * change API to take a point object Co-authored-by: tonyfwoo <55114329+tonyfwoo@users.noreply.github.com>
@MarshallOfSound has manually backported this PR to "8-x-y", please check out #21990 |
* feat: custom positioning for traffic light buttons (#21781) * feat: custom positioning for traffic light buttons * remove NSLog and unnecessary call-site in IsVisible() * no longer need to check if entering fullscreen * change API to take a point object Co-authored-by: tonyfwoo <55114329+tonyfwoo@users.noreply.github.com> * chore: add safety checks to RepositionTrafficLights Co-authored-by: Tony <TonyWuu@users.noreply.github.com> Co-authored-by: tonyfwoo <55114329+tonyfwoo@users.noreply.github.com>
I'm excited for this! Thanks! |
Description of Change
There seems to be a high demand for custom inset traffic light buttons in Electron.
BrowserWindow
,trafficLightPosition
which sets the(x,y)
position of the traffic light buttons.titleBarStyle: 'hidden'
to work.titleBarStyle: 'hidden'
.e.g.
Checklist
npm test
passesRelease Notes
Notes: add
trafficLightPosition
option inBrowserWindow
API to allow custom positioning of traffic lights