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

Fire button and shield button animations #476

Merged
merged 8 commits into from
Mar 25, 2022

Conversation

tomasstrba
Copy link
Contributor

@tomasstrba tomasstrba commented Mar 21, 2022

Task/Issue URL: https://app.asana.com/0/0/1201945397495594/f

Description:
Fire button and shield button animates if mouse cursor is over

Steps to test this PR:
Test fire button mouse over animation

  • please try in both dark and aqua mode

Test shield mouse over animation

  • please try in both dark and aqua mode
  • please try for both http and https connection
  • make sure the mouse over animation has a priority over automatic shield animation triggered together with the tracker animation

Internal references:

Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM

@@ -2738,6 +2755,29 @@
path = View;
sourceTree = "<group>";
};
AA7EB6EE27E880EA00036718 /* Animations */ = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved animations to their own subfolder. My apology for the harder readability of this PR because of this


@Published var isAnimationViewVisible = false

override var image: NSImage? {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case something sets the image while animation is in progress

import Lottie
import Combine

final class MouseOverAnimationButton: AddressBarButton {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This class encapsulates the mouse over animation. Developer only sets animationNames and class loads and runs animation as necessary.

@tomasstrba tomasstrba requested a review from mallexxx March 24, 2022 09:09
@tomasstrba tomasstrba assigned malbin and mallexxx and unassigned malbin Mar 24, 2022
Copy link
Collaborator

@mallexxx mallexxx left a comment

Choose a reason for hiding this comment

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

Looks good overall but I found a minor visual glitch (see attachment)

screencast.2022-03-25.16-13-15.mp4

@@ -417,7 +418,7 @@ extension AddressBarViewController {

if let point = self.view.mouseLocationInsideBounds(event.locationInWindow) {
guard self.view.window?.firstResponder !== addressBarTextField.currentEditor(),
!(self.view.hitTest(point) is NSButton)
!(self.view.hitTest(point) is NSButton || self.view.hitTest(point) is AnimationView)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest moving this as a var of a private NSView extension something like shouldShowArrowCursor in sake of effeciency and readability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion 👍 thanks

@mallexxx mallexxx assigned tomasstrba and unassigned mallexxx Mar 25, 2022
@tomasstrba tomasstrba assigned mallexxx and unassigned tomasstrba Mar 25, 2022
Copy link
Collaborator

@mallexxx mallexxx left a comment

Choose a reason for hiding this comment

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

LGTM. Works like a charm! 🔥

@tomasstrba tomasstrba merged commit 55d1cef into develop Mar 25, 2022
@tomasstrba tomasstrba deleted the tom/mouse-over-animations branch March 25, 2022 15:03
samsymons added a commit that referenced this pull request Mar 29, 2022
* develop:
  Prompt to store cards and identities (#461)
  Version 0.21.1
  inject CTL user script atDocumentEnd (#482)
  Fire button and shield button animations (#476)
  Add tds etag to breakage reports (#466)
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.

3 participants