Skip to content
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

Remove the border. Fix #48765 #48825

Closed
wants to merge 7 commits into from
Closed

Conversation

shizengzhou
Copy link
Contributor

@bpasero I remove the border to fix it.
border

@bpasero bpasero self-assigned this Apr 27, 2018
@bpasero bpasero added this to the May 2018 milestone Apr 27, 2018
@@ -41,7 +41,6 @@
cursor: pointer;
height: 35px;
box-sizing: border-box;
border: 1px solid transparent;
Copy link
Member

Choose a reason for hiding this comment

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

@shizengzhou not a good change, this border is needed when the tab.border color is set, see:

image

@bpasero bpasero modified the milestones: May 2018, On Deck Apr 30, 2018
@bpasero
Copy link
Member

bpasero commented Apr 30, 2018

Maybe we should conditionally set this border if tab.border is defined, I think that should be possible 👍

@bpasero bpasero modified the milestones: On Deck, April 2018, May 2018 Apr 30, 2018
@msftclas
Copy link

msftclas commented Apr 30, 2018

CLA assistant check
All CLA requirements met.

@shizengzhou
Copy link
Contributor Author

@bpasero I have changed my code.
tabs

@bpasero bpasero modified the milestones: May 2018, Backlog Apr 30, 2018
@bpasero
Copy link
Member

bpasero commented Apr 30, 2018

@shizengzhou I did not realize that we set a border color by default in dark and light theme and we cannot just remove it because the color is actually in use:

image

As you can see, there is a slight black line between the two tabs. I think a better approach would be to not use borders at all for this kind of separation and have it some other way...

@shizengzhou
Copy link
Contributor Author

@bpasero Now I don't use borders for the separations.
tabs

@bpasero bpasero modified the milestones: Backlog, May 2018 May 1, 2018
@bpasero bpasero modified the milestones: May 2018, June 2018 May 9, 2018
@bpasero
Copy link
Member

bpasero commented May 9, 2018

@shizengzhou how about keeping the border as is and letting the special case of having a border for the active tab use the ::pseudo element trick? I actually do not like how it currently uses box-shadow so then we could get rid of that hack too.

@bpasero
Copy link
Member

bpasero commented Jun 2, 2018

I found a simpler way via fd7d510

Thanks for the work here, sorry for not pushing it 👍

@bpasero bpasero closed this Jun 2, 2018
@shizengzhou shizengzhou deleted the 48765 branch June 2, 2018 09:16
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants