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

docs searchbox icon recoloring issues. #1165

Closed
the-j0k3r opened this issue Jul 5, 2020 · 11 comments
Closed

docs searchbox icon recoloring issues. #1165

the-j0k3r opened this issue Jul 5, 2020 · 11 comments
Labels
bug 🐛 Something isn't working

Comments

@the-j0k3r
Copy link
Member

the-j0k3r commented Jul 5, 2020

RE: discussion on a5ba294#commitcomment-40299609

link to svg https://docs.github.com/assets/images/octicons/search.svg

Im using

	body .ais-SearchBox-input {
		background: #181818 url('data:image/svg+xml;utf8,<svg width="17" height="16" viewBox="0 0 17 16" fill="none" xmlns="http://www.w3.org/2000/svg"><path fill-rule="evenodd" clip-rule="evenodd" d="M16.5089 13.3L12.7297 9.47C13.424 8.49 13.8307 7.3 13.8307 6C13.8307 2.69 11.1624 0 7.8791 0C4.5958 0 1.92749 2.69 1.92749 6C1.92749 9.31 4.5958 12 7.8791 12C9.16862 12 10.3391 11.59 11.3211 10.89L15.1202 14.7C15.3087 14.9 15.5666 15 15.8146 15C16.0626 15 16.3304 14.91 16.5089 14.7C16.8958 14.31 16.8958 13.68 16.5089 13.29V13.3ZM7.8791 10.7C5.30999 10.7 3.21701 8.59 3.21701 6C3.21701 3.41 5.30999 1.3 7.8791 1.3C10.4482 1.3 12.5412 3.41 12.5412 6C12.5412 8.59 10.4482 10.7 7.8791 10.7Z" fill="%23ccc"/></svg>') no-repeat 6px;
	}

Applying this actually removes icon fully why IDK.

GENERATOR ISSUES

Again, $color problems WHY are we generating this

GitHub-Dark/github-dark.css

Lines 10119 to 10121 in 161d652

body.d-lg-flex .ais-SearchBox-input {
background: #181818 url("/assets/images/octicons/search.svg") no-repeat 6px;
}

@silverwind

@the-j0k3r the-j0k3r added the bug 🐛 Something isn't working label Jul 5, 2020
@xt0rted
Copy link
Member

xt0rted commented Jul 5, 2020

@silverwind can correct me on this, but it's my understanding $color checks all properties for the mapped color and adds that rule/property to the generated style. The rule you're looking at is background: #fff url("/assets/images/octicons/search.svg") no-repeat 6px and we're mapping #fff to #181818 so it gets mapped and included in the output. Since it looks for just the color we don't have to add excessive mappings for all the different border styles, fonts, backgrounds, etc.

I'm pretty sure the intent of this is to create a color palette and let the mapper handle swapping them everywhere instead of just the manual mappings we used to have.

@the-j0k3r

This comment has been minimized.

@xt0rted
Copy link
Member

xt0rted commented Jul 5, 2020

If you look through https://docs.github.com/stylesheets/index.css there's duplicate rules in their file (can see it in dev tools too). Could that be part of the issue here? An example of this is:

Line 1068

.form-control,.ais-SearchBox-input,.form-select {
    min-height: 34px;
    padding: 6px 8px;
    font-size: 16px;
    line-height: 20px;
    color: #24292e;
    vertical-align: middle;
    background-color: #fff;
    background-repeat: no-repeat;
    background-position: right 8px center;
    border: 1px solid #d1d5da;
    border-radius: 3px;
    outline: none;
    box-shadow: inset 0 1px 2px rgba(27,31,35,.075)
}

.form-control.focus,.focus.ais-SearchBox-input,.form-control:focus,.ais-SearchBox-input:focus,.form-select.focus,.form-select:focus {
    border-color: #2188ff;
    outline: none;
    box-shadow: inset 0 1px 2px rgba(27,31,35,.075),0 0 0 .2em rgba(3,102,214,.3)
}

@media(min-width: 768px) {
    .form-control,.ais-SearchBox-input,.form-select {
        font-size:14px
    }
}

Line 9754

.form-control,.ais-SearchBox-input,.form-select {
    min-height: 34px;
    padding: 6px 8px;
    font-size: 16px;
    line-height: 20px;
    color: #24292e;
    vertical-align: middle;
    background-color: #fff;
    background-repeat: no-repeat;
    background-position: right 8px center;
    border: 1px solid #d1d5da;
    border-radius: 3px;
    outline: none;
    box-shadow: inset 0 1px 2px rgba(27,31,35,.075)
}

.form-control.focus,.focus.ais-SearchBox-input,.form-control:focus,.ais-SearchBox-input:focus,.form-select.focus,.form-select:focus {
    border-color: #2188ff;
    outline: none;
    box-shadow: inset 0 1px 2px rgba(27,31,35,.075),0 0 0 .2em rgba(3,102,214,.3)
}

@media(min-width: 768px) {
    .form-control,.ais-SearchBox-input,.form-select {
        font-size:14px
    }
}

Until generated styles get put in their own document sections there's always going to be issues, especially now that github.com is using a much different primer version than the other sites. They're doing a major overhaul on all this so they can add in themes (light, dark, high contrast) so at some point the other sites should get updated to the newer primer version but I'm sure they're focusing on just one site for now.

@the-j0k3r
Copy link
Member Author

They dont have conflicting values, the duplication shouldn't affect this. If it does I dont see how,

On the other hand on our part the specificity of these is being manipulated for one, not to mention the properties duplication and the rest.

in any case, the same goes on and on in out generated stuffs everywhere, not just here.

@xt0rted
Copy link
Member

xt0rted commented Jul 5, 2020

I'm sure once the generator adds document sections a lot of these issues will go away. We won't need the selector hacks and we won't have rules stepping on one another from the various sites.

@the-j0k3r
Copy link
Member Author

In meanwhile Im no closer to solving this issue when were going off on this tangent.

@the-j0k3r
Copy link
Member Author

the-j0k3r commented Jul 6, 2020

Ive removed the duplicates from the default stylesheet and its unrelated to this issue.

Also the issue is unrelated to sections or rules steeping over each other.

I dont think pasting those generated rules actually helped, please only refer to #1165 (comment) THAT is the issue..

@silverwind
Copy link
Member

Background generator is now smarter and only emits color in this case:

  body.d-lg-flex .ais-SearchBox-input, body.d-lg-flex .form-control,
  body.d-lg-flex .form-select {
    color: #cdcdcd;
    background-color: #181818;
    border-color: #404040;
  }

If the background longhand fails to parse (e.g. multiple backgrounds) it will still emit a shorthand because multiple background colors can not be represented in any other form in CSS.

@silverwind
Copy link
Member

Also, separate generator sections is on my todo list.

@the-j0k3r
Copy link
Member Author

Does anyone have anything to add that may help fix this? using the below

	body .ais-SearchBox-input {
		background: #181818 url('data:image/svg+xml;utf8,<svg width="17" height="16" viewBox="0 0 17 16" fill="none" xmlns="http://www.w3.org/2000/svg"><path fill-rule="evenodd" clip-rule="evenodd" d="M16.5089 13.3L12.7297 9.47C13.424 8.49 13.8307 7.3 13.8307 6C13.8307 2.69 11.1624 0 7.8791 0C4.5958 0 1.92749 2.69 1.92749 6C1.92749 9.31 4.5958 12 7.8791 12C9.16862 12 10.3391 11.59 11.3211 10.89L15.1202 14.7C15.3087 14.9 15.5666 15 15.8146 15C16.0626 15 16.3304 14.91 16.5089 14.7C16.8958 14.31 16.8958 13.68 16.5089 13.29V13.3ZM7.8791 10.7C5.30999 10.7 3.21701 8.59 3.21701 6C3.21701 3.41 5.30999 1.3 7.8791 1.3C10.4482 1.3 12.5412 3.41 12.5412 6C12.5412 8.59 10.4482 10.7 7.8791 10.7Z" fill="%23ccc"/></svg>') no-repeat 6px;
	}

instead of recoloring icon it removes it. can someone check that is correct?

@the-j0k3r
Copy link
Member Author

Fixed in own style, no interest in this anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants