Skip to content

Add pulse animation to assistant icon - #360

Merged
marcoambrosini merged 1 commit into
mainfrom
feature/359/animate-assistant-icon
Sep 18, 2025
Merged

Add pulse animation to assistant icon#360
marcoambrosini merged 1 commit into
mainfrom
feature/359/animate-assistant-icon

Conversation

@marcoambrosini

@marcoambrosini marcoambrosini commented Sep 15, 2025

Copy link
Copy Markdown
Member

fix #359

Screen.Recording.2025-09-17.at.09.39.17.mov

@marcoambrosini

Copy link
Copy Markdown
Member Author

Removed review request as icon size shrunk when I made the viewport bigger, looking for workaround

@jancborchardt jancborchardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks really nice! ✨

Signed-off-by: Marco Ambrosini <marcoambrosini@proton.me>
@marcoambrosini
marcoambrosini force-pushed the feature/359/animate-assistant-icon branch from a2c11f8 to a485671 Compare September 17, 2025 07:39
@marcoambrosini

Copy link
Copy Markdown
Member Author

Fixed the size issue, ready for review now

@marcelklehr marcelklehr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If an animation is necessary I like this better than the previous proposal. Looks nice :)

@jancborchardt jancborchardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems good to go then, right?

@marcoambrosini does it work gracefully when:

  • focusing instead of hover
  • stopping the hiver befire the animation is done (could just finish the animation)
  • staying on hovering (either is fine: only animate once, or repeat as well)

@marcoambrosini

Copy link
Copy Markdown
Member Author

@jancborchardt as of now focusing does not trigger the animation. Should I add it?

@jancborchardt

Copy link
Copy Markdown
Member

@marcoambrosini I'd say generally we should treat hover and focus as close as possible, so yes. But it's not a blocker.

@marcoambrosini

Copy link
Copy Markdown
Member Author

Gave it a quick try and this is a bit complex. I will reference in new issue

@marcoambrosini
marcoambrosini merged commit a699d03 into main Sep 18, 2025
9 checks passed
@github-project-automation github-project-automation Bot moved this from 🏗️ At engineering to 🎉 Done in 🖍 Design team Sep 18, 2025
@marcoambrosini
marcoambrosini deleted the feature/359/animate-assistant-icon branch September 18, 2025 14:33
@julien-nc

Copy link
Copy Markdown
Member

Do you want to add that to the NcAssistantIcon component of nextcloud/vue as well?

@marcoambrosini

Copy link
Copy Markdown
Member Author

That would result in this animation being in many other places right? I think it would be better to avoid that for now

@janepie janepie mentioned this pull request Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review enhancement New feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Hover animation of assistant button

4 participants