adobe-photoshop / spaces-design

Adobe Photoshop Design Space
http://adobe-photoshop.github.io/
Other
852 stars 74 forks source link

Animation for expanding/collapsing toolbar #3680

Open designedbyscience opened 8 years ago

designedbyscience commented 8 years ago

Another review only animation, this time for the toolbar. If an experimental flag is incoming, I can update this and my other animation PR to respect that.

iwehrman commented 8 years ago

The active-tool animation should probably be disabled when the toolbar is open: img

iwehrman commented 8 years ago

The pin/unpin toolbar animation is definitely another very bad case for the Spaces plugin because of the large amount of pixels that need to be painted in each frame, but I guess that's kind of the point...

iwehrman commented 8 years ago

It's easy to catch the animating icon rendering outside of the panel borders, which feels broken: img

iwehrman commented 8 years ago

Gripes: you asked for 'em, you got 'em.

designedbyscience commented 8 years ago

Updated, addressed gripes. Open question for @placegraphichere: How do you feel about the little animation that occurs now when the toolbar is NOT pinned and you switch tools via keyboard shortcuts?

iwehrman commented 8 years ago

It's working better now. I wouldn't say I love these animations overall. I guess it bothers me least when changing the pinned status of the toolbar. The animations that happen when switching tools in collapsed mode seem kind of busy and confusing to me.

placegraphichere commented 8 years ago

With the toolbar NOT pinned, it's only the selected tool that is visible, what I see when switching tools using the keyboard is that we're getting this little bounce when the tool is changed. But the results are not the same, since it depends whether the next tool is above or below the current one. So we get a bounce that could be higher in vertical, or lower. I think that seems odd.

In this tool selection state, referencing which direction the tool is coming from is not important. I think we should just assign a position to all non-selected tools (above or below the toolbar visible area), so the animation when changing tools is consistent.

Right now we're animating the UL into an index position, I think we should animate each tool individually. I took a quick stab at doing this, and the CSS is a little chunky, but I think it feels better.

toolbar-tool-animation2

ul {        
        transition: transform 0.1s linear;
        &.tool0 li:nth-child(1) {
            transform: translateY(0px);
        }
        &.tool1 li:nth-child(2) {
            transform: translateY(0);
        }
        &.tool2 li:nth-child(3) {
            transform: translateY(0);
        }
        &.tool3 li:nth-child(4) {
            transform: translateY(0);
        }
        &.tool4 li:nth-child(5) {
            transform: translateY(0);
        }
        &.tool5 li:nth-child(6) {
            transform: translateY(0);
        }
        &.tool6 li:nth-child(7) {
            transform: translateY(0);
        } 
        li {
            transition: transform 0.2s linear;
            transform: translateY(-4.8rem);
            position: absolute;
        }
}
&.expanded {
        height: 100%;
        color: @bg-alt;

        li {
            transform: translateY(0);
            position: relative;
        }
        .toolbar-button, .zoom {
            display: flex;
            opacity: 1;
        }        
    } 

var ulStyle,
                toolbarClassName = classnames({
                    "expanded": this.state.pinned || this.state.expanded,
                    "toolbar": true
                }),
                ulClassName;

            if (this.state.expanded || this.state.pinned) {

            } else {
                ulClassName = classnames({
                    "tool0": selectedToolIndex === 0,
                    "tool1": selectedToolIndex === 1,
                    "tool2": selectedToolIndex === 2,
                    "tool3": selectedToolIndex === 3,
                    "tool4": selectedToolIndex === 4,
                    "tool5": selectedToolIndex === 5,
                    "tool6": selectedToolIndex === 6
                });
            }

            return (
                <div className={toolbarClassName}>
                    <ul style={ulStyle} className={ulClassName}>
                        {tools}
                    </ul>
                    <Zoom />
                </div>
            );
designedbyscience commented 8 years ago

Okay...I think I can clean that up some, but keep the general premise of that animation