Skip to content

Add breadcrumb dropdown menu #249

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

Merged

Conversation

RayZhao1998
Copy link
Collaborator

@RayZhao1998 RayZhao1998 commented Mar 25, 2022

Description

This PR will support breadcrumbs dropdown menu mentioned in #248

Releated Issue

#248

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):

截屏2022-03-29 下午10 40 18

@0xWDG
Copy link
Collaborator

0xWDG commented Mar 25, 2022

Cool, just curious why are you wrapping menu inAnyView.

@RayZhao1998
Copy link
Collaborator Author

If not wrap with AnyView, it'll get a compiler error: Function declares an opaque return type, but the return statements in its body do not have matching underlying types

@lukepistrol lukepistrol linked an issue Mar 25, 2022 that may be closed by this pull request
@lukepistrol
Copy link
Member

If not wrap with AnyView, it'll get a compiler error: Function declares an opaque return type, but the return statements in its body do not have matching underlying types

Instead of wrapping in AnyView you could use the @ViewBuilder keyword on the function.

@ViewBuilder
private func menuItem(_ item: WorkspaceClient.FileItem) -> some View {
  if let children = item.children?.sortItems(foldersOnTop: true), !children.isEmpty {
    Menu {  }
  } else {
    Button {  }
  }
}

@RayZhao1998
Copy link
Collaborator Author

RayZhao1998 commented Mar 26, 2022

If not wrap with AnyView, it'll get a compiler error: Function declares an opaque return type, but the return statements in its body do not have matching underlying types

Instead of wrapping in AnyView you could use the @ViewBuilder keyword on the function.

@ViewBuilder
private func menuItem(_ item: WorkspaceClient.FileItem) -> some View {
  if let children = item.children?.sortItems(foldersOnTop: true), !children.isEmpty {
    Menu {  }
  } else {
    Button {  }
  }
}

Have tried that. Still got compiler error. Because this function will be called recursively.

截屏2022-03-26 下午4 50 55

@RayZhao1998 RayZhao1998 force-pushed the feature/breadcrumbs-dropdown-menu branch from 71341b5 to 0a52268 Compare March 26, 2022 10:02
@RayZhao1998
Copy link
Collaborator Author

Remove AnyView and extract menuItem to a component

@lukepistrol lukepistrol added enhancement New feature or request UI This is UI related labels Mar 26, 2022
@RayZhao1998 RayZhao1998 force-pushed the feature/breadcrumbs-dropdown-menu branch 3 times, most recently from 367ee1c to 743e8b3 Compare March 26, 2022 12:51
@RayZhao1998 RayZhao1998 marked this pull request as ready for review March 26, 2022 12:55
@RayZhao1998 RayZhao1998 requested a review from lukepistrol March 26, 2022 12:59
@lukepistrol
Copy link
Member

I guess colors inside the context menu aren't working for now

@lukepistrol
Copy link
Member

From testing this in a pretty barebones Node.js project with many subfolders in node_modules we get pretty heavy performance impact when opening the dropdown.

Untitled.mov

@RayZhao1998
Copy link
Collaborator Author

I guess colors inside the context menu aren't working for now

While in menu, colors of Text works.

@RayZhao1998
Copy link
Collaborator Author

From testing this in a pretty barebones Node.js project with many subfolders in node_modules we get pretty heavy performance impact when opening the dropdown.

Yep, I'm trying to optimize it.

@RayZhao1998
Copy link
Collaborator Author

It's a little difficult to optimize it based on SwiftUI's Menu, because it can't refresh view when data changes. I wanted to rewrite it with Popover but SwiftUI's Popover cannot remove the arrow.

@RayZhao1998 RayZhao1998 marked this pull request as draft March 27, 2022 13:35
@RayZhao1998
Copy link
Collaborator Author

RayZhao1998 commented Mar 27, 2022

Convert to draft. I'm working on rebuilding it with NSMenu.

@lukepistrol lukepistrol linked an issue Mar 28, 2022 that may be closed by this pull request
@RayZhao1998 RayZhao1998 force-pushed the feature/breadcrumbs-dropdown-menu branch 4 times, most recently from ca8f7e9 to 58f9cff Compare March 29, 2022 14:22
@RayZhao1998 RayZhao1998 force-pushed the feature/breadcrumbs-dropdown-menu branch from 58f9cff to caae42c Compare March 29, 2022 14:33
@RayZhao1998 RayZhao1998 marked this pull request as ready for review March 29, 2022 14:42
@austincondiff austincondiff merged commit 6138b4e into CodeEditApp:main Mar 29, 2022
xinix909 pushed a commit to xinix909/CodeTransfer that referenced this pull request Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request UI This is UI related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] - Breadcrumbs dropdown menu ✨ Path Bar
4 participants