GNOME Bugzilla – Bug 707565
appDisplay: Rework indicators animation
Last modified: 2013-09-16 16:57:06 UTC
Try to match mockup animation
Created attachment 254181 [details] [review] appDisplay: Rework indicators animation Previously the animation was not entirely according to the mockup. Now we are closer to the mockup.
Created attachment 254200 [details] [review] appDisplay: Rework indicators animation Previously the animation was not entirely according to the mockup. Now we are closer to the mockup.
Trying to do http://www.youtube.com/watch?list=UU4Ys3RMozzDVP6KoTjBTn4Q&feature=player_detailpage&v=5nJpKCJj2wQ#t=12 For the values, I saw the blend file of Jakub at: https://git.gnome.org/browse/gnome-shell-design/tree/mockups/motion/app-picker-groups.blend
Review of attachment 254200 [details] [review]: Not obviously wrong, but not a fan either - can't we achieve the desired animation by tweaking the existing parameters? ::: data/theme/gnome-shell.css @@ +944,2 @@ .page-indicator { + padding: 15px 20px; This looks like it belongs in the "remove padding" patch from the other bug? It doesn't look related to animations at all ... ::: js/ui/appDisplay.js @@ +239,3 @@ Tweener.addTween(children[i], { translation_x: 0, + time: INDICATORS_BASE_TIME + INDICATORS_ANIMATION_DELAY * i, Mmmh, linear animation time based on number of pages? So if there are nine pages, I have to wait a full second until I can switch to the last page?
(In reply to comment #4) > Review of attachment 254200 [details] [review]: > > Not obviously wrong, but not a fan either - can't we achieve the desired > animation by tweaking the existing parameters? > I don't think so, since we have to let each dot a different animation time to achieve that "curve". At least I tried with all the variation with the current implementation, and no, and it is logical that no, since each dot has the same animation time. > ::: data/theme/gnome-shell.css > @@ +944,2 @@ > .page-indicator { > + padding: 15px 20px; > > This looks like it belongs in the "remove padding" patch from the other bug? It > doesn't look related to animations at all ... > yeah, but it is. In fact, before I had that padding addition in the patch you propose, but, it belongs here, since it is part to achieve the animation like the mockup, if not the animation is too "quick" (and not because of the time but from the distance). Do you still want this in the other patch? > ::: js/ui/appDisplay.js > @@ +239,3 @@ > Tweener.addTween(children[i], > { translation_x: 0, > + time: INDICATORS_BASE_TIME + > INDICATORS_ANIMATION_DELAY * i, > > Mmmh, linear animation time based on number of pages? So if there are nine > pages, I have to wait a full second until I can switch to the last page? True, fixed in the upcoming set with a maximum time value (0.75s, and it's seeing beautiful still with 10 pages);
Created attachment 254633 [details] [review] appDisplay: Rework indicators animation Previously the animation was not entirely according to the mockup. Now we are closer to the mockup.
Created attachment 254646 [details] [review] appDisplay: Rework indicators animation Previously the animation was not entirely according to the mockup. Now we are closer to the mockup.
Created attachment 254879 [details] [review] appDisplay: Rework indicators animation Previously the animation was not entirely according to the mockup. Now we are closer to the mockup.
Review of attachment 254879 [details] [review]: Can't speak for code, but the result looks good to me.
Review of attachment 254879 [details] [review]: OK then. ::: data/theme/gnome-shell.css @@ +943,2 @@ .page-indicator { + padding: 15px 20px; I really dislike how the "correctness" of the animation depends on the padding :-(
(In reply to comment #10) > Review of attachment 254879 [details] [review]: > > OK then. > > ::: data/theme/gnome-shell.css > @@ +943,2 @@ > .page-indicator { > + padding: 15px 20px; > > I really dislike how the "correctness" of the animation depends on the padding > :-( (In reply to comment #10) > Review of attachment 254879 [details] [review]: > > OK then. > > ::: data/theme/gnome-shell.css > @@ +943,2 @@ > .page-indicator { > + padding: 15px 20px; > > I really dislike how the "correctness" of the animation depends on the padding > :-( Is not that bad if we let the padding as it is, but yeah, it is better with less padding, and when more padding is added, more ugly becomes (too much quick) so...lets push this at it is, and let people/designers try it day to day, and if they feel that the padding is too much little, then we will see what we can do =)
Attachment 254879 [details] pushed as 93d9c16 - appDisplay: Rework indicators animation
I put a comment in the commit message about the padding thing, just to make clear that *we* are not entirely sure with that and maybe we have to change it.