Skip to content

Fix animation (#45) #61

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.

Sign up for GitHub

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

Closed
0xWDG wants to merge 3 commits into from
Closed

Fix animation (#45) #61

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9beba9b
Fix animation (#45)
0xWDG Mar 18, 2022
7e5b3bf
Merge branch 'main' into main
0xWDG Mar 18, 2022
6dfa149
This should improve speed
0xWDG Mar 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions CodeEdit/SideBar/SideBarItem.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,15 @@ struct SideBarItem: View {

func sidebarFolderItem(_ item: WorkspaceClient.FileItem) -> some View {
DisclosureGroup(isExpanded: $isExpanded) {
if isExpanded { // Only load when expanded -> Improves performance massively
if !isExpanded {
// Only load the labels
ForEach(item.children!.sortItems(foldersOnTop: workspace.sortFoldersOnTop)) { child in
Label(child.url.lastPathComponent, systemImage: child.systemImage)
.accentColor(child.iconColor)
.font(.callout)
}
} else {
// Only load when expanded -> Improves performance massively
ForEach(item.children!.sortItems(foldersOnTop: workspace.sortFoldersOnTop)) { child in

@lukepistrol lukepistrol Mar 18, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually required since in a very large project folder this will load all children all the time which leads to very long file opening times.

@lukepistrol lukepistrol Mar 18, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen.Recording.2022-03-18.at.15.59.15.mov

@0xWDG 0xWDG Mar 18, 2022

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It breaks the animation.

Maybe it's an idea to only place the labels, and then update them?

@lukepistrol lukepistrol Mar 18, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can figure something out that doesn't break performance that'd be awesome. So far this if clause was the best option I found

@0xWDG 0xWDG Mar 18, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So instead of

SideBarItem(item: child,
workspace: workspace,
windowController: windowController)

We should use?

SideBarItem(item: child,
workspace: workspace,
windowController: windowController,
loadSubitems: isExpanded)

Or (maybe the worst solution)

Text(child.name) // Don't know is child.name exists. its pseudo code

@0xWDG 0xWDG Mar 18, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see, the issue is, when you open a file, it's rendering the whole file tree again.

This should

  1. be done on the background
  2. or not at all when opening a file

@lukepistrol lukepistrol Mar 18, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I suspected as well

@lukepistrol lukepistrol Mar 18, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I just tested the latest commit and it seems to run very nice. Couldn't see performance losses to before.

@0xWDG 0xWDG Mar 18, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, after some research, opening/closing doesn't trigger an update.
selecting a file does.

There seems to be a delay again when opening files from that video

This has been fixed with commit 6dfa149

6dfa149 (for refrerence, clickable)

This commit increases the speed, but this is not the solution, the problem is that this bar is getting re-rendered when you open/select a file.
if you disable this if, and only open and close the disclosure groups, then it works as expected.

@lukepistrol lukepistrol Mar 18, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SwiftUI acts strangely sometimes…

SideBarItem(item: child,
workspace: workspace,
Expand All @@ -54,6 +62,6 @@ struct SideBarItem: View {
Label(item.url.lastPathComponent, systemImage: item.systemImage)
.accentColor(.secondary)
.font(.callout)
}
}
}
}