Skip to content

Custom(izable) Navigator Sidebar Implementation #231

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
merged 11 commits into from
Mar 25, 2022

Conversation

lukepistrol
Copy link
Member

@lukepistrol lukepistrol commented Mar 24, 2022

Description

The main problem of SwiftUI's implemented sidebar is, that it is not very customizable and only brings native selection state when using NavigationLink. This has the downside of opening a whole new detail view every time a file is selected from the sidebar.

The new Sidebar uses Lists selection property to get the current selected file and set the document in the Code Editor

Things that need to be fixed:

  • When right clicking on a file or folder which is not currently selected, no blue border appears while the contextMenu is shown. (help wanted)
  • Localization of contextMenu
  • corner radius of selected sidebar item needs to be slightly larger
  • make colors right

Releated Issue

#123

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:

Group 1
Group 2
Group 3

Screen.Recording.2022-03-24.at.17.24.54.mov

@lukepistrol lukepistrol added enhancement New feature or request UI This is UI related help wanted Extra attention is needed labels Mar 24, 2022
@lukepistrol lukepistrol linked an issue Mar 24, 2022 that may be closed by this pull request
12 tasks
@austincondiff
Copy link
Collaborator

austincondiff commented Mar 24, 2022

Rounded corners need to be more rounded, 5px I believe
image

@austincondiff
Copy link
Collaborator

Also I think the blue or gray highlight has a blend mode applied. Give me a second and I will find that out for you.

@austincondiff
Copy link
Collaborator

austincondiff commented Mar 24, 2022

Here is the list selection background specification (to my best understanding).

Light Mode Focused Selection Color

Color Opacity Blending Mode
#0A82FF 75% normal
#0A82FF 100% soft light

Dark Mode Focused Selection Color

Color Opacity Blending Mode
#0A82FF 55% normal
#0A82FF 100% soft light

Light Mode Unfocused Selection Color

Color Opacity Blending Mode
#000000 11% normal

Dark Mode Unfocused Selection Color

Color Opacity Blending Mode
#FFFFFF 12% normal

Other considerations

  • This focused selection color is only applied when sidebar is in focus, the unfocused selection color is applied when sidebar is not in focus or window is inactive.
  • Be sure to use the users preferred color in place of the blue for the hue.
  • We may want to make this reusable in some way

References

To gather the above information, I referred to Apple's own Sketch design resource.

image

Screen Shot 2022-03-24 at 12 28 24 PM

Screen Shot 2022-03-24 at 12 28 39 PM

Despite my findings, I am seeing some inconsistency when I apply this in Sketch vs Figma. I have been searching to find if Apple provides a variable to get this exact background but haven't found anything yet. I have posted in the Apple developer forums here to see if anyone else has any insight on this.

@lukepistrol
Copy link
Member Author

@austincondiff updated the colors & corner radius. also adjusted the spacing to match Xcodes sidebar.

@austincondiff
Copy link
Collaborator

austincondiff commented Mar 25, 2022

@lukepistrol can we keep the sidebar named "NavigatorSidebar"? I would like to reference our sidebars by function (navigator, inspector - this is what Xcode calls them) rather than location (leading, trailing). If you have a particular reason why we shouldn't do that I am open to it though!

@RayZhao1998
Copy link
Collaborator

RayZhao1998 commented Mar 25, 2022

@lukepistrol can we keep the sidebar named "NavigatorSidebar"? I would like to reference our sidebars by function (navigator, inspector - this is what Xcode calls them) rather than location (leading, trailing). If you have a particular reason why we shouldn't do that I am open to it though!

Because I think the leading sidebar contains multiple functions(navigator, search...)

You can see more here

@RayZhao1998
Copy link
Collaborator

RayZhao1998 commented Mar 25, 2022

Oh sorry. I found that Xcode's leading sidebar is called 'Navigator'. My bad.😭

截屏2022-03-25 下午8 37 00

So you can rename LeadingSidebar -> NavigatorSidebar and NavigatorSidebar -> ProjectSidebar

@lukepistrol
Copy link
Member Author

So you can rename LeadingSidebar -> NavigatorSidebar and NavigatorSidebar -> ProjectSidebar

I will rename like so:

  • Navigator Sidebar
    • ProjectNavigator
    • FindNavigator (Previously: Search)

So that it matches Xcodes description

@lukepistrol lukepistrol marked this pull request as ready for review March 25, 2022 14:36
Copy link
Collaborator

@austincondiff austincondiff left a comment

Choose a reason for hiding this comment

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

Outstanding work! One last thing I just noticed, when the window is inactive, the icons go to 50% opacity, at least I believe it is 50%, so do the sidebar dock icons - compare to Xcode.

@lukepistrol
Copy link
Member Author

Outstanding work! One last thing I just noticed, when the window is inactive, the icons go to 50% opacity, at least I believe it is 50%, so do the sidebar dock icons - compare to Xcode.

I think it's more like 45%

Copy link
Collaborator

@RayZhao1998 RayZhao1998 left a comment

Choose a reason for hiding this comment

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

Fantastic! LGTM

@lukepistrol
Copy link
Member Author

@austincondiff just need your approval then I'll merge

@lukepistrol lukepistrol removed the help wanted Extra attention is needed label Mar 25, 2022
@lukepistrol lukepistrol merged commit a6eced1 into CodeEditApp:main Mar 25, 2022
@lukepistrol lukepistrol deleted the new-sidebar branch March 25, 2022 15:34
@austincondiff
Copy link
Collaborator

Just pulled. it doesn't quite match your screenshots

image

@lukepistrol
Copy link
Member Author

lukepistrol commented Mar 25, 2022

Just pulled. it doesn't quite match your screenshots

Oops. Forgot to update them. Spacing still needs to be fixed.
Haven't found a way yet to change this with the list implementation.

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.

✨ Custom implementation of Sidebar
3 participants