Skip to content

Conversation

lilingxi01
Copy link
Member

@lilingxi01 lilingxi01 commented Apr 22, 2022

Description

  • Fix breadcrumbs spacing issue.
  • Optimize the style of breadcrumb separator to better match the Xcode.
  • Add inactive colors to Tabs and Breadcrumbs.

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
  • I documented my code
  • Review requested

Screenshots

CleanShot 2022-04-22 at 22 16 24@2x

CleanShot 2022-04-22 at 22 15 48@2x

CleanShot 2022-04-22 at 22 26 22@2x

CleanShot 2022-04-22 at 22 26 50@2x

@Angelk90
Copy link
Contributor

@lilingxi01 : Can you take a screenshot of when there is no file open?

@lukepistrol
Copy link
Member

@lilingxi01 : Can you take a screenshot of when there is no file open?

When no file is open neither the tab bar nor the breadcrumbs bar is visible. So this is completely unrelated.

@austincondiff
Copy link
Collaborator

austincondiff commented Apr 22, 2022

Comparing the two, Xcode seems to be using blue folder icons. There is even less space between the end of the label and the > symbol. Compare the s in cypress with the s in Breadcrumbs in your screenshot. The text label for both Tabs and Breadcrumbs are also a bit lighter when inactive relatively speaking. Looking good so far!

@austincondiff
Copy link
Collaborator

Can you update the screenshot?

@lilingxi01
Copy link
Member Author

@austincondiff : I'm still trying to fix the folder icon color issue. I currently cannot find any NSColor that is similar to the Xcode's one.

@austincondiff
Copy link
Collaborator

They're probably using the same color as the macOS folder icon.

@austincondiff
Copy link
Collaborator

austincondiff commented Apr 22, 2022

Try this if there is not an NSColor for it:
Dark mode: RGB 97 182 223
Light mode: RGB 39 185 255

@austincondiff
Copy link
Collaborator

dark-inactive-tab-fix

Please change the inactive native tab background to 45% black instead of 50% black in dark mode.

@austincondiff
Copy link
Collaborator

austincondiff commented Apr 22, 2022

I also noticed Apple uses shadows instead of a hard border and the font is a tiny bit lighter in weight...

image
image

Sorry to be so nit-picky. These tabs are going to be so good, thank you for all you're doing on this!

@austincondiff
Copy link
Collaborator

austincondiff commented Apr 22, 2022

I'd like to get your opinion. In Finder, Safari, etc, a native tab activates on press. In Xcode, a tab activates on release. Do you think we should activate a tab on press only if displaying native tabs?

I guess Safari does this when you change tab style, it is hard to tell but when using separate tabs it activates on press, and when I am using unified tabs it activates on release...

Screen.Recording.2022-04-22.at.4.52.39.PM.mp4

What do you think?

@lilingxi01
Copy link
Member Author

@austincondiff : I think active on release makes more sense and feels better.

@lilingxi01
Copy link
Member Author

Current style of tab bar and breadcrumbs:

CleanShot 2022-04-22 at 22 16 24@2x

CleanShot 2022-04-22 at 22 15 48@2x

@lilingxi01
Copy link
Member Author

Current style of Inactive States:

CleanShot 2022-04-22 at 22 22 30@2x

CleanShot 2022-04-22 at 22 23 07@2x

@lilingxi01
Copy link
Member Author

lilingxi01 commented Apr 23, 2022

Shadow Compare

CleanShot 2022-04-22 at 22 30 59@2x

CleanShot 2022-04-22 at 22 28 01@2x

@lilingxi01
Copy link
Member Author

lilingxi01 commented Apr 23, 2022

And, tab divider now goes under the shadow (and top/bottom divider):

CleanShot 2022-04-22 at 22 35 29@2x

CleanShot 2022-04-22 at 22 35 03@2x

@lilingxi01
Copy link
Member Author

lilingxi01 commented Apr 23, 2022

@lukepistrol : This PR has already finished updating. Ready for next review! 🎉

@lilingxi01 lilingxi01 requested a review from lukepistrol April 23, 2022 02:52
@austincondiff austincondiff self-requested a review April 23, 2022 07:09
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.

Looks really good @lilingxi01, very impressive. Glad we were able to get those details ironed out! 👏

@lukepistrol lukepistrol merged commit d996bbc into CodeEditApp:main Apr 23, 2022
@lilingxi01 lilingxi01 deleted the tabs-breadcrumbs-fix branch April 23, 2022 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐞 Too much spacing in breadcrumb bar 🐞 Implement Inactive States In UI

4 participants