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

Add simple slide-in animation for the tool-bar #306

Closed
wants to merge 1 commit into from
Closed

Add simple slide-in animation for the tool-bar #306

wants to merge 1 commit into from

Conversation

ericcornelissen
Copy link
Contributor

Based on the discussion in #300, this adds a simple slide-in animation for the tool-bar. Below is a preview of what this looks like at 20fps (the animation works both when you start/reload Atom and when you change the position of the tool-bar).

preview

Implementation

I used @keyframes to create the animations. The animations are based on margins with exact pixel values given the configured tool-bar icon size.

If the animations are created starting from, e.g. margin-left: -100% the toolbar is hidden, but the panel is not and so the animation does not work. If the animations are created using the CSS transform property (I tried both scale() and translate()) you have the same problem.

I'm not sure how this animation works if the toolbar somehow has multiple rows/columns...

I used @Keyframes to create the animations. The animations are based on 
margins with exact pixel values given the configured tool-bar icon size.

If the animations are created starting from, e.g. `margin-left: -100%` 
the toolbar is hidden, but the panel is not and so the animation does 
not work.

If the animations are created using the CSS `transform` property (I 
tried both `scale()` and `translate()`) you have the same problem.

I'm not sure how this animation works if the toolbar somehow has 
multiple rows/columns...
@aminya
Copy link
Member

aminya commented Jun 7, 2020

This looks laggy and jumpy on my computer. It looks worse than the default behavior! At least the default behavior was a single frame appearance. This is like 2-3 for me, and gets stuck in between the frames! I am not sure why (maybe because I use an integrated graphics card).

Setting animation-duration: 1.2s; makes it smoother, but that is a lot of delay time!

I feel like that #300 is a better solution.

@ericcornelissen
Copy link
Contributor Author

This looks laggy and jumpy on my computer. It looks worse than the default behavior! At least the default behavior was a single frame appearance. This is like 2-3 for me, and gets stuck in between the frames! I am not sure why (maybe because I use an integrated graphics card).

Hmmm, that's unfortunate... Would you mind trying to replace margin-left: -100px by transform: translateX(-100px) (or the correct translation for wherever your toolbar is positioned). That might result in better performance.

Setting animation-duration: 1.2s; makes it smoother, but that is a lot of delay time!

Not a fan of that either, no 😅

I feel like that #300 is a better solution.

If the above doesn't work, I agree that it is at least a better solution than this.

@aminya
Copy link
Member

aminya commented Jun 10, 2020

This looks laggy and jumpy on my computer. It looks worse than the default behavior! At least the default behavior was a single frame appearance. This is like 2-3 for me, and gets stuck in between the frames! I am not sure why (maybe because I use an integrated graphics card).

Hmmm, that's unfortunate... Would you mind trying to replace margin-left: -100px by transform: translateX(-100px) (or the correct translation for wherever your toolbar is positioned). That might result in better performance.

using transform: translateY(-100px) and animation-duration: 0.5s; makes it better. However, the toolbar still appears quite late compared to #300.

I think here we are doing things that are not common in other software. All the usual software, load their toolbar normally during the loading time. We don't need to circumvent doing this! Tool-bar is in my opinion a crucial part of Atom, and it is so useful that I even like to see it included in Atom by default!

@ericcornelissen
Copy link
Contributor Author

using transform: translateY(-100px) and animation-duration: 0.5s; makes it better.

Better? does that mean "it looks good enough to be potentially merged"?

However, the toolbar still appears quite late compared to #300.

That makes sense, given the activation hook 😉

I think here we are doing things that are not common in other software. All the usual software, load their toolbar normally during the loading time. We don't need to circumvent doing this!

That is a fair opinion that I can sympathize with. And given this argument, I'm much more inclined to agree with you.

Personally, I would prefer it to load later because I know it is a possibility. But to be fair, if I didn't know it was an option I wouldn't mind at either. It would be nice if we could just toggle this all-together but I don't think that's a possibility.

All-in-all: given this argument I'm okay with merging #300 instead of this, but in the end it's up to @suda.

Tool-bar is in my opinion a crucial part of Atom, and it is so useful that I even like to see it included in Atom by default!

I personally disagree, and I think so does the Atom team. (It's not at all a core feature of an editor. Sublime doens't have it, Jetbrain's IDE's don't have this kind of toolbar nor does Visual Studio. Only VS Code has it, as far as I know, and some webbased IDEs that are, I believe, based on VS code). But you could propose it (if it hasn't been proposed yet).

@aminya
Copy link
Member

aminya commented Jul 8, 2020

@ericcornelissen Should we close this in favor of #300 ?

@aminya aminya mentioned this pull request Jul 8, 2020
@ericcornelissen ericcornelissen deleted the toolbar-animation branch July 29, 2020 09:16
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.

2 participants