Closed Bug 1513228 Opened 5 years ago Closed 3 years ago

Remove or simplify expensive saved-to-library animation

Categories

(Firefox :: Bookmarks & History, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Tracking Status
firefox66 --- wontfix
firefox90 --- fixed

People

(Reporter: dao, Assigned: dao)

References

Details

(Keywords: perf, Whiteboard: fixed by bug 1704865)

Attachments

(1 obsolete file)

See bug 1506996 comment 11.

This animation is mostly redundant with the "Saved to Library!" hint, so I think we can just remove this.
Summary: Remove expensive save-to-library animation → Remove expensive saved-to-library animation
Depends on: 1459907
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Amy, how do you feel about this?

I don't think we can fix this animation the same way the tracking protection animation was fixed in bug 1506996, as we're dealing with a very wide range of possible toolbar button icon colors with different OSes and settings.
Flags: needinfo?(amlee)
(In reply to Dão Gottwald [::dao] from comment #1)
> I don't think we can fix this animation the same way the tracking protection
> animation was fixed in bug 1506996, as we're dealing with a very wide range
> of possible toolbar button icon colors with different OSes and settings.

There's other avenues we can explore. I just filed bug 1513670 with a suggestion.
(In reply to Dão Gottwald [::dao] from comment #1)
> Amy, how do you feel about this?
> 
> I don't think we can fix this animation the same way the tracking protection
> animation was fixed in bug 1506996, as we're dealing with a very wide range
> of possible toolbar button icon colors with different OSes and settings.

Hi, 

I'd prefer if we were able to keep the animation if possible, or a modified version of it that is more performant rather than removing it altogether. Does Markus' suggestion help with this?
Flags: needinfo?(amlee)
(In reply to Amy Lee [:amylee] UX from comment #4)
> (In reply to Dão Gottwald [::dao] from comment #1)
> > Amy, how do you feel about this?
> > 
> > I don't think we can fix this animation the same way the tracking protection
> > animation was fixed in bug 1506996, as we're dealing with a very wide range
> > of possible toolbar button icon colors with different OSes and settings.
> 
> Hi, 
> 
> I'd prefer if we were able to keep the animation if possible, or a modified
> version of it that is more performant rather than removing it altogether.
> Does Markus' suggestion help with this?

I don't know if anyone has the time to evaluate and implement that solution for this particular animation... I certainly won't anytime soon.

Another way to solve the perf problem would be to animate the shapes only without the color change.

However, others have mentioned at the all-hands that the number of animations around bookmarking (first when clicking the star, then when saving) are a bit much and probably not that helpful, so I think we should seriously consider removing this one.
Flags: needinfo?(amlee)
If the colour transition is the issue, would this issue apply to other animations as well? (i.e Save to Pocket, Downloads, Move to Overflow Menu, etc.).
Flags: needinfo?(amlee) → needinfo?(dao+bmo)
(In reply to Amy Lee [:amylee] UX from comment #6)
> If the colour transition is the issue, would this issue apply to other
> animations as well? (i.e Save to Pocket, Downloads, Move to Overflow Menu,
> etc.).

Yes, except for Downloads, where I think we change the color without a transition.
Flags: needinfo?(dao+bmo) → needinfo?(amlee)
(In reply to Dão Gottwald [::dao] from comment #7)
> (In reply to Amy Lee [:amylee] UX from comment #6)
> > If the colour transition is the issue, would this issue apply to other
> > animations as well? (i.e Save to Pocket, Downloads, Move to Overflow Menu,
> > etc.).
> 
> Yes, except for Downloads, where I think we change the color without a
> transition.

If that is the case then I am more hesitant to agree to removing this animation since it sounds like all colour transitions would be removed from the other icons as well beyond bookmarking, is that correct?  

I would like to understand the amount of performance gain from doing this? If the issue is the actual colour transition I would like to explore alternatives (i.e does shortening the number of transitional frames make a difference?) since if we change the colour transition for bookmarking this will need to be changed for the other icons as well.
Flags: needinfo?(amlee) → needinfo?(dao+bmo)
(In reply to Amy Lee [:amylee] UX from comment #8)
> (In reply to Dão Gottwald [::dao] from comment #7)
> > (In reply to Amy Lee [:amylee] UX from comment #6)
> > > If the colour transition is the issue, would this issue apply to other
> > > animations as well? (i.e Save to Pocket, Downloads, Move to Overflow Menu,
> > > etc.).
> > 
> > Yes, except for Downloads, where I think we change the color without a
> > transition.
> 
> If that is the case then I am more hesitant to agree to removing this
> animation since it sounds like all colour transitions would be removed from
> the other icons as well beyond bookmarking, is that correct?  

No, the implementations are separate. We'll want some solution for each one of them but not necessarily the same.

> I would like to understand the amount of performance gain from doing this?

If done right, the animation would run on the compositor, ideally on the GPU, such that CPU usage won't spike when the animation runs.

> If the issue is the actual colour transition I would like to explore
> alternatives (i.e does shortening the number of transitional frames make a
> difference?)

It might mitigate it but not really solve it.

> since if we change the colour transition for bookmarking this
> will need to be changed for the other icons as well.

That's not the case.

Performance aside, can you also please comment on the UX side of things here? We currently run two animations in parallel: the "Saved to Library!" hint with the checkmark animation, and the library icon animation. Surely two animations competing for the user's attention isn't ideal?
Flags: needinfo?(dao+bmo) → needinfo?(amlee)
From a performance perspective, I should note that these animations are a lot less worrisome than the tracking protection animation. The tracking protection animation runs on nearly every pageload, whereas these animations only run in response to a user interaction with the particular UI element.

Nobody has noticed the performance problem of the bookmarks animation in profiles.

(In reply to Dão Gottwald [::dao] from comment #5)
> However, others have mentioned at the all-hands that the number of
> animations around bookmarking (first when clicking the star, then when
> saving) are a bit much and probably not that helpful, so I think we should
> seriously consider removing this one.

Can you have the relevant people weigh in on this bug? Otherwise it's hard to determine whether there's actual consensus among UX about this.
(In reply to Markus Stange [:mstange] from comment #10)
> From a performance perspective, I should note that these animations are a
> lot less worrisome than the tracking protection animation. The tracking
> protection animation runs on nearly every pageload, whereas these animations
> only run in response to a user interaction with the particular UI element.
> 
> Nobody has noticed the performance problem of the bookmarks animation in
> profiles.

Sure, it's something that usually only happens once in a blue moon. Still suboptimal and wasteful.

> (In reply to Dão Gottwald [::dao] from comment #5)
> > However, others have mentioned at the all-hands that the number of
> > animations around bookmarking (first when clicking the star, then when
> > saving) are a bit much and probably not that helpful, so I think we should
> > seriously consider removing this one.
> 
> Can you have the relevant people weigh in on this bug?

I'm not sure, I think it was mak and ntim... or possibly mak and Paolo or standard8, but I think mak was involved.

> Otherwise it's hard
> to determine whether there's actual consensus among UX about this.

Umm, I didn't mean to imply that there's some kind of consensus among UX. I actually expect that decisions are delegated and not every UX decision needs consensus across the whole group. For the time being I just want Amy's thoughts since she made the spec for bug 1459877.
Hi, 

Actually Aaron had created the original spec for the door-hanger redesign. In the original animation there was no confirmation pop-up and that was added alongside the door-hanger update. I can't remember the details on why that was added afterwards (it could be that users still didn't know where their bookmark went). NI Aaron.
Flags: needinfo?(amlee) → needinfo?(abenson)
Summary: Remove expensive saved-to-library animation → Remove or simplify expensive saved-to-library animation
(In reply to Amy Lee [:amylee] UX from comment #12)
> Hi, 
> 
> Actually Aaron had created the original spec for the door-hanger redesign.
> In the original animation there was no confirmation pop-up and that was
> added alongside the door-hanger update. I can't remember the details on why
> that was added afterwards (it could be that users still didn't know where
> their bookmark went). NI Aaron.

Hi Dao, 

I spoke to Aaron and we can simplify the animation by removing the "Saved to Library!" portion of it. It doesn't sound like there is a serious performance issue with keeping bookmarking animation (star dropping into the library icon).
Flags: needinfo?(abenson)
(In reply to Amy Lee [:amylee] UX from comment #13)
> (In reply to Amy Lee [:amylee] UX from comment #12)
> > Hi, 
> > 
> > Actually Aaron had created the original spec for the door-hanger redesign.
> > In the original animation there was no confirmation pop-up and that was
> > added alongside the door-hanger update. I can't remember the details on why
> > that was added afterwards (it could be that users still didn't know where
> > their bookmark went). NI Aaron.
> 
> Hi Dao, 
> 
> I spoke to Aaron and we can simplify the animation by removing the "Saved to
> Library!" portion of it.

I'm confused because you earlier suggested that the confirmation popup might have been added because the library animation wasn't clear enough. Was that not the case? I've heard before that many users don't understand the library icon in the first place, so I'm surprised to hear that the popup can go and the animated icon is good enough.

> It doesn't sound like there is a serious
> performance issue with keeping bookmarking animation (star dropping into the
> library icon).

Actually that's the only part with a performance issue. It's not as serious because this animation doesn't happen frequently while browsing the web, but it still doesn't meet our performance expectations and can contribute to making Firefox look bad with users that care about battery life etc.
Flags: needinfo?(amlee)
(In reply to Dão Gottwald [::dao] from comment #5)
> Another way to solve the perf problem would be to animate the shapes only
> without the color change.

Another option would be to color the icon and stop animating the shape. This would only make sense in combination with the confirmation popup.
(In reply to Dão Gottwald [::dao] from comment #14)
> (In reply to Amy Lee [:amylee] UX from comment #13)
> > (In reply to Amy Lee [:amylee] UX from comment #12)
> > > Hi, 
> > > 
> > > Actually Aaron had created the original spec for the door-hanger redesign.
> > > In the original animation there was no confirmation pop-up and that was
> > > added alongside the door-hanger update. I can't remember the details on why
> > > that was added afterwards (it could be that users still didn't know where
> > > their bookmark went). NI Aaron.
> > 
> > Hi Dao, 
> > 
> > I spoke to Aaron and we can simplify the animation by removing the "Saved to
> > Library!" portion of it.
> 
> I'm confused because you earlier suggested that the confirmation popup might
> have been added because the library animation wasn't clear enough. Was that
> not the case? 
This was a speculative comment and I spoke to Aaron to confirm if this was the case but there hasn't been any evidence to suggest this was an issue.

I've heard before that many users don't understand the library
> icon in the first place, so I'm surprised to hear that the popup can go and
> the animated icon is good enough.


Can you provide more detail of where you have gotten this feedback from? We are currently talking about the animation and not the library icon itself. I would treat this as a separate discussion. We had performed user testing of this animation (without the pop-up confirmation) to verify there wasn't any major issues around clarity and I had spoken to Aaron to confirm if there was any follow-up feedback for this animation.



> 
> > It doesn't sound like there is a serious
> > performance issue with keeping bookmarking animation (star dropping into the
> > library icon).
> 
> Actually that's the only part with a performance issue. It's not as serious
> because this animation doesn't happen frequently while browsing the web, but
> it still doesn't meet our performance expectations and can contribute to
> making Firefox look bad with users that care about battery life etc.

The colour transition also applies to "Save to Pocket" and moving items into the overflow menu. We cannot change the colour transition without applying the same changes to the other icons as to maintain consistency. As I said earlier, I am hesitant to okay the removal of the highlight colour as I think this does impact UX. The highlight colour directs the user's attention and removing that makes it less noticeable if we only had animation, especially on large monitors. I'm not sure if this is a worthwhile trade-off. I would be happy to work with the performance team to come up with an alternative solution if a performance fix cannot be found (i.e if it's the transition states from colour A to colour B that is the issue then this can possibly be removed).
Flags: needinfo?(amlee)
(In reply to Amy Lee [:amylee] UX from comment #16)
> (In reply to Dão Gottwald [::dao] from comment #14)
> > (In reply to Amy Lee [:amylee] UX from comment #13)
> > > (In reply to Amy Lee [:amylee] UX from comment #12)
> > > > Hi, 
> > > > 
> > > > Actually Aaron had created the original spec for the door-hanger redesign.
> > > > In the original animation there was no confirmation pop-up and that was
> > > > added alongside the door-hanger update. I can't remember the details on why
> > > > that was added afterwards (it could be that users still didn't know where
> > > > their bookmark went). NI Aaron.
> > > 
> > > Hi Dao, 
> > > 
> > > I spoke to Aaron and we can simplify the animation by removing the "Saved to
> > > Library!" portion of it.
> > 
> > I'm confused because you earlier suggested that the confirmation popup might
> > have been added because the library animation wasn't clear enough. Was that
> > not the case? 
> This was a speculative comment and I spoke to Aaron to confirm if this was
> the case but there hasn't been any evidence to suggest this was an issue.

So how did the confirmation hint end up in the design? There must have been some good reason?

> I've heard before that many users don't understand the library
> > icon in the first place, so I'm surprised to hear that the popup can go and
> > the animated icon is good enough.
> 
> 
> Can you provide more detail of where you have gotten this feedback from?

I don't remember but I think Stephen Horlander is aware of the issue.

> > > It doesn't sound like there is a serious
> > > performance issue with keeping bookmarking animation (star dropping into the
> > > library icon).
> > 
> > Actually that's the only part with a performance issue. It's not as serious
> > because this animation doesn't happen frequently while browsing the web, but
> > it still doesn't meet our performance expectations and can contribute to
> > making Firefox look bad with users that care about battery life etc.
> 
> The colour transition also applies to "Save to Pocket" and moving items into
> the overflow menu. We cannot change the colour transition without applying
> the same changes to the other icons as to maintain consistency.

Can we drop the color transition from all these then? I.e. just change the color immediately? Note that we use the attention color for the download button without transitioning that color change.
Flags: needinfo?(amlee)
> So how did the confirmation hint end up in the design? There must have been some good reason?

This was an oversight on my part and I didn't fully consider the collision between the bookmark star animation and the tooltip animation. The intent behind the tooltip was to draw attention to and clarify _where_ the bookmark is going but the star animation already provides this cue. My apologies for the added confusion.
Priority: -- → P3
Can we shorten number of frames for the colour transition? - The fade-out duration from highlight colour to grey is deliberately extended in the design. The entire animation is smooth and to have an abrupt colour switch breaks that animation flow, removing it entirely would be a last resort if this is causing serious issues but we can definitely shorten the transition to have less frames and extend the duration of the highlight colour as an alternative. The download animation is a different interaction than bookmarks, pocket, and overflow. The highlight colour is used to tell the user that the download is done. A transition in that situation isn't appropriate since you want to know right away when your download is finished so you can access your file. In the case of the others, we are indicating to the user where their item is located when they want to access it later so the immediacy is not there.
Flags: needinfo?(amlee) → needinfo?(dao+bmo)
(In reply to Amy Lee [:amylee] UX from comment #19)
> Can we shorten number of frames for the colour transition? - The fade-out
> duration from highlight colour to grey is deliberately extended in the
> design. The entire animation is smooth and to have an abrupt colour switch
> breaks that animation flow, removing it entirely would be a last resort if
> this is causing serious issues but we can definitely shorten the transition
> to have less frames and extend the duration of the highlight colour as an
> alternative.

How about we defer the fade-out animation until after the shape animation? It would be less expensive then and we'd be less constrained with regards to the number of frames and smoothness, as we wouldn't re-paint the entire shape-animation sprite for every frame.
Flags: needinfo?(dao+bmo) → needinfo?(amlee)
(In reply to Dão Gottwald [::dao] from comment #20)
> (In reply to Amy Lee [:amylee] UX from comment #19)
> > Can we shorten number of frames for the colour transition? - The fade-out
> > duration from highlight colour to grey is deliberately extended in the
> > design. The entire animation is smooth and to have an abrupt colour switch
> > breaks that animation flow, removing it entirely would be a last resort if
> > this is causing serious issues but we can definitely shorten the transition
> > to have less frames and extend the duration of the highlight colour as an
> > alternative.
> 
> How about we defer the fade-out animation until after the shape animation?
> It would be less expensive then and we'd be less constrained with regards to
> the number of frames and smoothness, as we wouldn't re-paint the entire
> shape-animation sprite for every frame.

Is the issue on fade-out only? (highlight colour to grey)?
Flags: needinfo?(amlee) → needinfo?(dao+bmo)
No, it's an issue both ways. I was implicitly suggesting that we drop the fade-in transition. Let me prototype this and see how it feels...
Flags: needinfo?(dao+bmo)
Depends on: 1704865
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Whiteboard: fixed by bug 1704865
Attachment #9030653 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: