X Tutup
The Wayback Machine - https://web.archive.org/web/20221117040039/https://github.com/CodeEditApp/CodeEdit/pull/225
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

Remove Tab Removal Animation #225

Closed
wants to merge 7 commits into from

Conversation

thecoolwinter
Copy link
Collaborator

@thecoolwinter thecoolwinter commented Mar 23, 2022

Description

Removes the default animation when removing tabs from the tab list. By default SwiftUI adds a 0.35 second animation to items, but this makes for an unresponsive and slow tab bar. I've removed the animation by adding an .animation(nil, value: UUID()) to each tab item in the TabBar.swift file.

Releated Issue

Checklist (for drafts):

  • I read and understood the contributing guide as well as the code of conduct
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • Review requested

Screenshots (if appropriate):

Before (with animation):
https://user-images.githubusercontent.com/35942988/159793534-18dc09ab-7411-4c0b-8997-f01e4ea95726.mov

After (no animation):
https://user-images.githubusercontent.com/35942988/159793576-f1299a64-3bef-4a05-af46-8a270b1b6d35.mov

@austincondiff
Copy link
Collaborator

austincondiff commented Mar 23, 2022

The default tabs in macOS are animated. We need to find a way to make the tabs more performant rather than removing the animation.

Screen.Recording.2022-03-23.at.4.40.28.PM.mov

@thecoolwinter
Copy link
Collaborator Author

thecoolwinter commented Mar 23, 2022

Ah you're right. I did some experimenting and found that replacing the HStack in the tab bar with a LazyHStack and providing an explicit height to the stack made it a tad more performant. I also added a small animation duration to the tab. Does this look good?

I also did some digging and found that there may be a memory leak somewhere with highlightr. I think that may be part of where the performance issues are coming from. The tabs themselves are performant (if I remove the editor from the window the tabs move just find), but the editor is taking compute and memory.

For now this provides an explicit animation + fade to the tabs and replaces them with a LazyHStack which should scale to tons of tabs easily. Is this okay to add?

@austincondiff
Copy link
Collaborator

austincondiff commented Mar 24, 2022

Switching tabs should not close one file and open another. Right now it is essentially replacing file contents in the editor every time we switch tabs which could be expensive. We should really be caching our open files so when we switch between tabs we are not having to re-compute syntax highlighting among other things. I think this will ultimately increase performance when switching tabs.

We need to make sure the tab behavior is identical to macOS default tabs with precision whatever that means. That includes animation duration and easing. I notice when you close a tab on the end it fades out which I don’t believe macOS tabs do. I’ve seen the entire content view fade in certain scenarios which is not done in macOS either.

@austincondiff austincondiff added this to Blocked / Needs Clarification in Release 1.0 Mar 25, 2022
@lukepistrol lukepistrol linked an issue Mar 28, 2022 that may be closed by this pull request
@lukepistrol lukepistrol added the clarification needed Further information is requested label Mar 29, 2022
@lukepistrol lukepistrol marked this pull request as draft Mar 29, 2022
@lukepistrol lukepistrol added the WIP This is work-in-progress label Apr 1, 2022
@austincondiff
Copy link
Collaborator

austincondiff commented Apr 4, 2022

@thecoolwinter this PR is falling behind in commits. What do we want to do here? The animation needs to stay but we do need to get a little closer to macOS style tabs in terms of animation. We also need to be able to reorder them (#256).

@austincondiff
Copy link
Collaborator

austincondiff commented Apr 18, 2022

Closing in favor of #461

Release 1.0 automation moved this from Blocked / Needs Clarification to Done Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification needed Further information is requested WIP This is work-in-progress
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Get rid of tab close animation
3 participants
X Tutup