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

feat: expose new vibrancy types #19073

Merged
merged 1 commit into from Jul 11, 2019

Conversation

Projects
None yet
2 participants
@codebytere
Copy link
Member

commented Jul 2, 2019

Description of Change

In macOS Mojave, Apple added a host of new NSVisualEffectMaterials. This PR enabled developers to use those new types by adding them to options for new BrowserWindows and when invoking the setVibrancy API on BrowserWindow instances.

cc @MarshallOfSound @nornagon @miniak

Checklist

Release Notes

Notes: Added support for NSVisualEffectMaterials vibrancy types added in macOS Mojave.

@ckerr
Copy link
Member

left a comment

Manually comparing to 20+ different strings and static_casting the result seems pretty crude.

Is this done because the macOS version is determined at runtime s.t. we can't really build a static list of type strings to values at compile time?

@codebytere

This comment has been minimized.

Copy link
Member Author

commented Jul 2, 2019

@ckerr potentially! i agree it's pretty crude but it was done so that we don't need to forward-declare everything; i can try to see if there's a better solution but i'm not sure one exists at present 🤔

@electron-cation electron-cation bot removed the new-pr 🌱 label Jul 3, 2019

@ckerr

This comment has been minimized.

Copy link
Member

commented Jul 3, 2019

Keeping it working on older macOSes is more important than slightly tidier code.

@codebytere

This comment has been minimized.

Copy link
Member Author

commented Jul 3, 2019

@ckerr i'm a little unclear on which solution you're advocating for; do you mean you think we should forward declare these enum values?

@codebytere codebytere force-pushed the add-new-vibrancies branch from 49e49c6 to 14b3a7f Jul 4, 2019

@zcbenz zcbenz requested a review from ckerr Jul 11, 2019

@ckerr

ckerr approved these changes Jul 11, 2019

@codebytere codebytere force-pushed the add-new-vibrancies branch from 14b3a7f to 0b53814 Jul 11, 2019

@codebytere codebytere force-pushed the add-new-vibrancies branch from 0b53814 to 0cbdc7a Jul 11, 2019

@codebytere

This comment has been minimized.

Copy link
Member Author

commented Jul 11, 2019

app module app.requestSingleInstanceLock passes arguments to the second-instance event

on Windows and

chromium feature command line switches --remote-debugging-port switch should display the discovery page - should display the discovery page

on Linux

are unrelated and should not block merge.

@codebytere codebytere merged commit 75a020e into master Jul 11, 2019

9 of 13 checks passed

appveyor: win-ia32-testing AppVeyor build failed
Details
build-linux Workflow: build-linux
Details
electron-arm64-testing Build #20190711.21 had test failures
Details
Artifact Comparison Changes Detected
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
build-mac Workflow: build-mac
Details
electron-arm-testing Build #20190711.21 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented Jul 11, 2019

Release Notes Persisted

Added support for NSVisualEffectMaterials vibrancy types added in macOS Mojave.

@codebytere codebytere deleted the add-new-vibrancies branch Jul 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.