feat: add optional animation parameter to BrowserWindow.setVibrancy #35987
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
codebytere
added
semver/minor
codebytere
changed the title
zcbenz
reviewed
Oct 12, 2022
shell/browser/native_window_mac.mm
Outdated
| __weak auto weak_delegate = window_delegate_.get(); | ||
| [NSAnimationContext | ||
| runAnimationGroup:^(NSAnimationContext* context) { | ||
| context.duration = 0.3; |
There was a problem hiding this comment.
A hard coded value might not be enough for all apps, some apps might want a slow fading in animation for startup effect, and some might want a short animation for things like notifications.
There was a problem hiding this comment.
added an animationDuration prop to the options object on the API
docs/api/browser-window.md
Outdated
|
|
||
| * `type` string | null - Can be `appearance-based`, `light`, `dark`, `titlebar`, | ||
| `selection`, `menu`, `popover`, `sidebar`, `medium-light`, `ultra-dark`, `header`, `sheet`, `window`, `hud`, `fullscreen-ui`, `tooltip`, `content`, `under-window`, or `under-page`. See | ||
| the [macOS documentation][vibrancy-docs] for more details. | ||
| * `animate` boolean (optional) |
There was a problem hiding this comment.
An options object might be better since there are lots of other vibrancy related things that could be added in future.
There was a problem hiding this comment.
I agree with @zcbenz. This should be an options object for future proofing.
There was a problem hiding this comment.
refactored API to use options object
gerhardberger
force-pushed
the
add-window-vibrancy-animation
branch
from
fd154d9
to
c72452d
Compare
samuelmaddock
suggested changes
Nov 14, 2022
docs/api/browser-window.md
Outdated
|
|
||
| * `type` string | null - Can be `appearance-based`, `light`, `dark`, `titlebar`, | ||
| `selection`, `menu`, `popover`, `sidebar`, `medium-light`, `ultra-dark`, `header`, `sheet`, `window`, `hud`, `fullscreen-ui`, `tooltip`, `content`, `under-window`, or `under-page`. See | ||
| the [macOS documentation][vibrancy-docs] for more details. | ||
| * `options` boolean (optional) | ||
| * `animate` boolean (optional) - Whether to animate or not the vibrancy toggle. | ||
| * `animationDuration` number (optional) - The duration of the animation in seconds. |
There was a problem hiding this comment.
As far as I know, this is the first introduction of an API accepting a unit of time in Electron. I think it would be best to use milliseconds—as commonly used in DOM APIs—and possibly update our best practices guidelines to reflect this.
There was a problem hiding this comment.
got it, refactored to milliseconds
|
Build failure in mac tests |
nornagon
requested changes
Nov 15, 2022
gerhardberger
force-pushed
the
add-window-vibrancy-animation
branch
from
5abba2b
to
310ca9f
Compare
nornagon
reviewed
Dec 12, 2022
gerhardberger
force-pushed
the
add-window-vibrancy-animation
branch
from
310ca9f
to
743368f
Compare
nornagon
reviewed
Dec 20, 2022
electron-cation
bot
added
api-review/approved ✅
and removed
api-review/requested 🗳
labels
zcbenz
force-pushed
the
add-window-vibrancy-animation
branch
from
82c1fde
to
d67707c
Compare
|
@nornagon Can you take another look at this PR? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds an optional animate Boolean parameter to BrowserWindow.setVibrancy method so when applying a vibrant backdrop for a window or removing it, it fades in/out accordingly.
animate=false:before.mov
animate=true:after.mov
Release Notes
Notes: Add optional animation parameter to BrowserWindow.setVibrancy