8000 Not wanted Web Profiler Toolbar link color css override · Issue #27658 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

Not wanted Web Profiler Toolbar link color css override #27658

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.

Sign up for GitHub

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

Jump to bottom
Closed
soullivaneuh opened this issue Jun 20, 2018 · 13 comments
Closed

Not wanted Web Profiler Toolbar link color css override #27658

soullivaneuh opened this issue Jun 20, 2018 · 13 comments
Labels
Bug Help wanted Issues and PRs which are looking for volunteers to complete them. Status: Needs Review WebProfilerBundle

Comments

@soullivaneuh
Copy link
Contributor

Symfony version(s) affected: v4.0.11

Description

If I put some CSS rules with a body classes, it will apply on the web profiler toolbar.

How to reproduce

body.logon {
  a:not(.btn) {
    color: map_get($project-colors, 'red');

    &:hover {
      text-decoration: underline;
    }
  }
}

And create a template with <body class="logon">

You will have something like this:

image

I'm pretty sure it's related to the body class case, because I have another global color definition for a links and it does not affect the toolbar.
@stof
Copy link
Member
stof commented Jun 20, 2018

Well, you final selector is body.logon a:not(.btn). That has a higher specificity than the rules defined in the WDT styles. So it wins over them.

the WDT being in your page, CSS rules will indeed impact them. I don't know of a way to isolate the WDT from your page (using an iframe to render the toolbar is likely to break things entirely)

@soullivaneuh
Copy link
Contributor Author

I don't know of a way to isolate the WDT from your page

I'm not a CSS expert, but inline css on the bar would do the tric 8000 k, would not do? 🤔

Or maybe some !important tag, but it's quite ugly.

Maybe my sass code could be better too, but I don't know how to do otherwise.

@stof
Copy link
Member
stof commented Jun 20, 2018

But we would also have to reset all styles coming from your CSS (and there are thousands of CSS properties). So inline styles would become huge (and unmaintainable).
And then, they would also win over the WDT styles (making it impossible to use CSS cascading features in the WDT, as well as hover styles).
!important suffers from most of these issues (we could still use some of the CSS features by applying !important on all WDT styles).

Thus, inline CSS styles are as ugly as !important IMO, and they are not compatible with CSP.

Maybe my sass code could be better too, but I don't know how to do otherwise.

First step to reduce the specificity would be to use .logon instead of body.logon. That might be enough to go below the specificity of the WDT styles in some cases (not sure for this one though).

another idea could be to apply logon class not on <body> but on a <div> wrapping all your own HTML (but the WDT would be injected after this div, not inside it, and so not match the rule).
And you could also accept the fact that the link in the WDT goes red. That's not totally broken.

@ro0NL
Copy link
Contributor
ro0NL commented Jun 20, 2018

Could https://www.w3schools.com/cssref/css3_pr_all.asp help?

@stof
Copy link
Member
stof commented Jun 20, 2018

no, because the more specific rule would still allow setting the color on the button over WDT styles resetting with all.

@soullivaneuh
Copy link 8000
Contributor Author

@stof I will try your two suggestion and give a feedback, thanks.

And you could also accept the fact that the link in the WDT goes red. That's not totally broken.

Nah, I'm perturbed with that. 😜

@ro0NL
Copy link
Contributor
ro0NL commented Jun 21, 2018

could it be related to the fact WDT styling is applied on class level, opposed to id. The latter seems to have higher precedence.

https://jsfiddle.net/kvh0wubd/

@soullivaneuh
Copy link
Contributor Author

Good catch @ro0NL!

It seems to be indeed better to rely on an id.

@soullivaneuh
Copy link
Contributor Author

For my case, using .logon instead of body.logon on my sass file partially solve the issue.

Links under boxes are blue again, but the underline effect on hover is still present on link under the .sf-toolbar-block divs.

I made things simpler by adding an id on my first layout div: <div id="logon" class="container"> and rely on it for my CSS custom rules.

I let the issue open. I think the @ro0NL's case is interesting to give a try. 👍

Feel free to close if you think nothing more have to be done here.

@javiereguiluz
Copy link
Member

The thing we could try is to apply all: initial !important; to all the important elements of the debug toolbar and see if that overrides any external style applied to it. Any volunteer to test this? Thanks!

@javiereguiluz javiereguiluz added the Help wanted Issues and PRs which are looking for volunteers to complete them. label Jun 24, 2018
@alcalyn
Copy link
Contributor
alcalyn commented Jun 28, 2018 •
edited
Loading

TLDR: Do you think it is worth using a css reset for the debug profiler bar?

I already used the all: initial / all: unset trick as described here: https://stackoverflow.com/a/31317986/2960951

But it also reset a lot of css default rules, like box-sizing, display: block of paragraph, titles, divs, headers, the appearance of radio buttons... Here is how the full css reset I used looks like:

// Full reset styles
& {
    all: initial;
    * {
        all: unset;
    }
}

// Normalize
*, ::before, ::after {
    box-sizing: content-box;
}

h1, h2, h3, h4, h5, h6, div, header, footer {
    display: block;
}

input[type="button"], button {
    box-sizing: border-box;
}

input[type="radio"] {
    appearance: radio;
    -moz-appearance: radio;
    -webkit-appearance: radio;
    display: inline-block;
}

footer {
    padding: 0;
}

a {
    cursor: pointer;
}

This code is actually used in a website with a lot of visits. I use this to fully isolate css of a plugin that displays popins on many websites, and prevent websites which are displaying popins "break" popins style with their css rules.

It works well but IMHO, it is a big thing for something that impacts someting in dev environment only.

For your use case, you could avoid adding a class on <body> tag, and instead adding your class on another tag as you actually did:

<body>
    <main class="logon">
    </main>
</body>

With the css rule main.logon {}.

Do you think it is worth using a css reset for the debug profiler bar?

This was referenced Jun 28, 2018
Closed
Closed
Merged
@alcalyn
Copy link
Contributor
alcalyn commented Jun 28, 2018

Assuming that links color can be too easily overriden by application css, this PR set a stronger precedence for links css only. #27758

fabpot added a commit that referenced this issue Jul 5, 2018
@fabpot
bug #27758 [WebProfilerBundle] Prevent toolbar links color override b…
46a32f1 
…y css (alcalyn)

This PR was merged into the 2.8 branch.

Discussion
----------

[WebProfilerBundle] Prevent toolbar links color override by css

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27658
| License       | MIT
| Doc PR        |

Fixes this issue: #27658 (comment)

Links color in toolbar can be easily override by application css. As this could happens sometimes, this PR set links color with a stronger CSS precedence.

Commits
-------

e12e217  Prevent toolbar links color override by css
@fabpot fabpot closed this as completed Jul 5, 2018
@soullivaneuh
Copy link
Contributor Author

I'm sorry, but I still have the issue with v4.2.5:

profiler_link_override

a:not(.btn):not(.close):not([data-dt-idx]):not(.page-link), button:not(.btn):not(.close) {
    color: #2f333d;
    -webkit-text-decoration: underline dotted #e14a4b;
    text-decoration: underline dotted #e14a4b;
    transition: all 0.3s ease-in-out;
}

The color is also overriding the link on sub-content like ajax list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Help wanted Issues and PRs which are looking for volunteers to complete them. Status: Needs Review WebProfilerBundle
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@fabpot @javiereguiluz @stof @ro0NL @alcalyn @soullivaneuh @xabbuh @carsonbot
0