镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

New implementation of Project Navigator - Performance Improvements - #411

Merged
lukepistrol merged 18 commits into
CodeEditApp:mainfrom
lukepistrol:bug/proj-nav-outline-view
Apr 8, 2022
Merged

lukepistrol merged 18 commits into
CodeEditApp:mainfrom
lukepistrol:bug/proj-nav-outline-view

Conversation

@lukepistrol

@lukepistrol lukepistrol commented Apr 6, 2022 •

Copy link
Copy Markdown
Member

Description

This is still WIP.

I implemented a NSViewControllerRepresentable hosting a NSOutlineView in order to fix performance issues on very large folders.

As you can see below there are no delays at all when unfolding a folder which took up to a couple of seconds as discussed here: #395.

TODOs:

  • UI fine adjustments
  • context menu (right click)
  • Documentation

Out of scope for now:

  • Reload list when folder content changes

Related Issue

Checklist

  • 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

Screen Shot 2022-04-07 at 11 19 40

Screen.Recording.2022-04-07.at.11.19.54.mov

@lukepistrol lukepistrol added bug Something isn't working performance This is performance related navigator labels Apr 6, 2022
@lukepistrol lukepistrol self-assigned this Apr 6, 2022
@0xWDG

0xWDG commented Apr 6, 2022

Copy link
Copy Markdown
Collaborator

Looks great @lukepistrol!

@lukepistrol lukepistrol added the WIP This is work-in-progress label Apr 6, 2022
@Angelk90

Angelk90 commented Apr 6, 2022

Copy link
Copy Markdown
Contributor

@lukepistrol : Ok, I got to test it seems to work fine.

I would like to hear @RayZhao1998 opinion, I could take a look at the statistics.
Understanding when the situation has improved compared to before.

@RayZhao1998

Copy link
Copy Markdown
Collaborator

It looks great!

@lukepistrol
lukepistrol marked this pull request as ready for review April 7, 2022 09:58
@lukepistrol lukepistrol changed the title [WIP] New implementation of Project Navigator - Performance Improvements New implementation of Project Navigator - Performance Improvements Apr 7, 2022
@lukepistrol lukepistrol removed the WIP This is work-in-progress label Apr 7, 2022
@RayZhao1998

Copy link
Copy Markdown
Collaborator

I've tested this. It's awesome!

A little bug: if I open a file whose folder is not expanded from breadcrumbs, the selection state will not change.
For example. .all-contributorsrc has been opened. Now I open .circleci/config.yml from breadcrumbs and .circleci is not expanded. The selection is still .all-contributorsrc.

截屏2022-04-07 9 59 24 PM

@lukepistrol
lukepistrol marked this pull request as draft April 7, 2022 15:00
@lukepistrol
lukepistrol marked this pull request as ready for review April 8, 2022 13:08
@lukepistrol lukepistrol changed the title [WIP] New implementation of Project Navigator - Performance Improvements New implementation of Project Navigator - Performance Improvements Apr 8, 2022
@lukepistrol lukepistrol removed the WIP This is work-in-progress label Apr 8, 2022

@nanashili nanashili left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see any issues. Looks good @lukepistrol

@RayZhao1998

RayZhao1998 commented Apr 8, 2022 •

Copy link
Copy Markdown
Collaborator

When I open a new project, the workspace folder is not expanded by default now. Also workspace icon color is missing.
截屏2022-04-08 9 46 47 PM

if row == -1 {
outlineView.deselectRow(outlineView.selectedRow)
}
outlineView.selectRowIndexes(.init(integer: row), byExtendingSelection: false)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I did a quick experiment. You can do like this to expand current selection file's folder.

var parent = item.parent
while parent != nil {
    outlineView.expandItem(parent)
    parent = parent?.parent
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah it works great but it introduces another problem: When you collapse the folder of the selected item and then expand another folder, the folder of the selected item gets expanded as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think I'll leave it for now and we can figure something out in a later PR.

@lukepistrol

Copy link
Copy Markdown
Member Author

When I open a new project, the workspace folder is not expanded by default now.

I mean Xcode does this too when fist opening a project you never opened before. But I can change it to auto expand the first level.

@lukepistrol

Copy link
Copy Markdown
Member Author

@lukepistrol : Is there a possibility to have the icons and text similar in size to that of xcode?

It actually is the exact size

Group 7

At the moment it appears that the following implementation is not supported.

Yeah I know

@Angelk90

Angelk90 commented Apr 8, 2022 •

Copy link
Copy Markdown
Contributor

@lukepistrol :

Some considerations:

  • Is there a possibility to have the icons and text similar in size to that of xcode?

Schermata 2022-04-08 alle 16 29 10

Schermata 2022-04-08 alle 16 29 22

  • At the moment it appears that the following implementation is not supported.

Schermata 2022-04-08 alle 16 32 02

  • When you switch from one file to another, you will see the selected file in the left sidebar if the folder is open.
    Instead of being the same color as the selection (blue in my case), it could be a different color gray to mean a different value, like xcode does.
Registrazione.schermo.2022-04-08.alle.16.35.10.mov
Registrazione.schermo.2022-04-08.alle.16.39.25.mov
  • Now it has also stopped working properly.
Registrazione.schermo.2022-04-08.alle.16.43.25.mov

@lukepistrol

Copy link
Copy Markdown
Member Author

When you switch from one file to another, you will see the selected file in the left sidebar if the folder is open.
Instead of being the same color as the selection (blue in my case), it could be a different color gray to mean a different value, like xcode does.

This is because the editor view is not becoming first responder once a file is opened. Not in scope of the project navigator!

@Angelk90

Angelk90 commented Apr 8, 2022

Copy link
Copy Markdown
Contributor

@lukepistrol : This happens to me, the elements no longer keep the selection, see both videos.

Registrazione.schermo.2022-04-08.alle.16.47.51.mov

Continue:

Registrazione.schermo.2022-04-08.alle.16.52.30.mov

@lukepistrol

Copy link
Copy Markdown
Member Author

@Angelk90 selection should now work as expected

@Angelk90

Angelk90 commented Apr 8, 2022

Copy link
Copy Markdown
Contributor

@lukepistrol : Seems not to be a problem at the moment.
To have the bigger icons, would it be to have this thing in the settings?

@lukepistrol
lukepistrol requested a review from RayZhao1998 April 8, 2022 16:11
@nanashili

Copy link
Copy Markdown
Contributor

How do you guys get bugs but when I test it it's fine?!

@lukepistrol

Copy link
Copy Markdown
Member Author

How do you guys get bugs but when I test it it's fine?!

They only occurred when you opened files, then closed them all together and then reopened a file. Was a small recursion problem 😄

@Angelk90

Angelk90 commented Apr 8, 2022

Copy link
Copy Markdown
Contributor

How do you guys get bugs but when I test it it's fine?!

@nanashili : There we invent them. 😂

@RayZhao1998 RayZhao1998 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! It's awesome!

@lukepistrol
lukepistrol merged commit 8be6e8f into CodeEditApp:main Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working navigator performance This is performance related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants