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

[Profiler] Fix dark theme elements color #30860

Merged
merged 1 commit into from
Apr 6, 2019

Conversation

dFayet
Copy link

@dFayet dFayet commented Apr 4, 2019

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

I think the color handling of the code highlighted (exceptions, ...) is quite questionnable, but as we use the highlight_file() function, I don't really see an other way to handle it, tell me what you think.

Before/After

Form
Capture d’écran de 2019-04-04 10-07-18

Capture d’écran de 2019-04-04 10-03-27

Twig render graph
Capture d’écran de 2019-04-04 10-07-36

Capture d’écran de 2019-04-06 19-37-02

Exceptions
Capture d’écran de 2019-04-04 10-08-19

Capture d’écran de 2019-04-06 09-39-27

Doctrine queries
Capture d’écran de 2019-04-04 10-08-37

Capture d’écran de 2019-04-04 10-04-47

@fabpot
Copy link
Member

fabpot commented Apr 4, 2019

Can you paste some before/after screenshots?

@stof
Copy link
Member

stof commented Apr 4, 2019

The red text on dark background for the exception message does not have enough contrast to be readable (and this gets even worse for color-blind people). As that's the most important part of the content, that should be changed.

@nicolas-grekas nicolas-grekas added this to the 4.2 milestone Apr 4, 2019
@@ -183,6 +186,12 @@
h3.form-data-type + h3 {
margin-top: 1em;
}
.theme-dark .toggle-icon {
background-image: url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAgCAYAAAAbifjMAAAKrWlDQ1BpY2MAAEiJlZcHUJPZFsfv96U3WgLSCb0jvUoJPRRBOtgISSChhJiCitgRV3AtiIiAuqCrFAVFKbJWLFhYFOx1gywK6rpYsKHmfcgj+N7Mmzd7Zs53f3Pm3v859869M+cDgPyRJRRmwyoA5AgkopgQf3pScgodNwggAAMCoAM7FlssZERHRwDEpsYfDALg/e2JLwA3bCe0wD8zVQ5XzEZkohFO44jZOQgfQ1zGFookAKDKkLjxYolwgjsQpomQAhHumeCMSZZNcNokv/s+Jy4mAAA0HgA8mcUSZQBApiFxeh47A9EhOyNsL+DwBQhzEPZh81jISN6HsE1OTu4EX0fYIu0HnYz/0ExTaLJYGQqe3Mt3wwfyxcJs1tJ/eBz/33KypVM5zBAn80ShMRP5kDO7m5UbrmBB2uyoKeZzJmuaYJ40NH6K2eKAlCnmsALDFWuzZ0dMcTo/mKnQkTDjppgrDoqdYlFujCJXuiiAMcUs0XReaVa8Is7jMhX6+by4xCnO4yfMnmJxVmz49JwARVwkjVHUzxWE+E/nDVbsPUf8w375TMVaCS8uVLF31nT9XAFjWlOcpKiNww0Mmp4Tr5gvlPgrcgmzoxXzudkhirg4L1axVoJcyOm10YozzGSFRU8x4INIwAJsCXeJZKL4gFzhUhE/gyehM5BXxaUzBWw7G7qjvYMHABNvdPIKjF77/vYgLdXp2Lq1yJVeJ5fL26ZjzBoAjnwDgOQ4HTNvAUC5AIBLgWypKG8yhp74YAARKAMa0AL6wBhYAFvgCFyBF/ADQSAMRIE4kAwWADbggRwgAotBAVgNikAJ2AK2g0qwB+wFdeAQaAHt4AQ4Cy6Cq+A6uAUeABkYAi/AKHgPxiEIwkEUiAppQQaQKWQNOULukA8UBEVAMVAylAplQAJIChVAa6ESqBSqhGqgeugIdBw6C12G+qB70AA0Ar2BPsMomAzTYD3YDJ4Ju8MMOByOg+fDGfAiOB8uhDfBFXAtfBBug8/CV+FbsAx+AY+hAIqE0kAZomxR7qgAVBQqBZWOEqFWoIpR5ahaVBOqE9WNuoGSoV6iPqGxaCqajrZFe6FD0fFoNnoRegV6I7oSXYduQ59H30APoEfR3zAUjC7GGuOJYWKSMBmYxZgiTDlmP6YVcwFzCzOEeY/FYjWw5lg3bCg2GZuJXYbdiN2FbcaewfZhB7FjOBxOC2eN88ZF4Vg4Ca4ItxN3EHca148bwn3Ek/AGeEd8MD4FL8CvwZfjG/Cn8P34Z/hxggrBlOBJiCJwCEsJmwn7CJ2Ea4QhwjhRlWhO9CbGETOJq4kVxCbiBeJD4lsSiWRE8iDNIfFJq0gVpMOkS6QB0ieyGtmKHECeR5aSN5EPkM+Q75HfUigUM4ofJYUioWyi1FPOUR5TPipRleyUmEocpZVKVUptSv1Kr5QJyqbKDOUFyvnK5cpHla8pv1QhqJipBKiwVFaoVKkcV7mjMqZKVXVQjVLNUd2o2qB6WXVYDadmphakxlErVNurdk5tkIqiGlMDqGzqWuo+6gXqEA1LM6cxaZm0EtohWi9tVF1N3Vk9QX2JepX6SXWZBkrDTIOpka2xWaNF47bG5xl6MxgzuDM2zGia0T/jg6aOpp8mV7NYs1nzluZnLbpWkFaW1latdq1H2mhtK+052ou1d2tf0H6pQ9Px0mHrFOu06NzXhXWtdGN0l+nu1e3RHdPT1wvRE+rt1Dun91JfQ99PP1O/TP+U/ogB1cDHgG9QZnDa4Dldnc6gZ9Mr6Ofpo4a6hqGGUsMaw17DcSNzo3ijNUbNRo+MicbuxunGZcZdxqMmBiaRJgUmjSb3TQmm7qY80x2m3aYfzMzNEs3Wm7WbDZtrmjPN880bzR9aUCx8LRZZ1FrctMRaultmWe6yvG4FW7lY8ayqrK5Zw9au1nzrXdZ9NhgbDxuBTa3NHVuyLcM2z7bRdsBOwy7Cbo1du92rmSYzU2Zundk985u9i322/T77Bw5qDmEOaxw6Hd44WjmyHascbzpRnIKdVjp1OL12tnbmOu92vutCdYl0We/S5fLV1c1V5NrkOuJm4pbqVu12x53mHu2+0f2SB8bD32OlxwmPT56unhLPFs+/vWy9srwavIZnmc/izto3a9DbyJvlXeMt86H7pPr84iPzNfRl+db6PvEz9uP47fd7xrBkZDIOMl752/uL/Fv9PwR4BiwPOBOICgwJLA7sDVILig+qDHocbBScEdwYPBriErIs5EwoJjQ8dGvoHaYek82sZ46GuYUtDzsfTg6PDa8MfxJhFSGK6IyEI8Mit0U+nG06WzC7PQpEMaO2RT2KNo9eFP3bHOyc6DlVc57GOMQUxHTHUmMXxjbEvo/zj9sc9yDeIl4a35WgnDAvoT7hQ2JgYmmiLGlm0vKkq8nayfzkjhRcSkLK/pSxuUFzt88dmucyr2je7fnm85fMv7xAe0H2gpMLlReyFh5NxaQmpjakfmFFsWpZY2nMtOq0UXYAewf7BcePU8YZ4XpzS7nP0r3TS9OHM7wztmWM8Hx55byX/AB+Jf91ZmjmnswPWVFZB7Lk2YnZzTn4nNSc4wI1QZbgfK5+7pLcPqG1sEgoW+S5aPuiUVG4aL8YEs8Xd0hoSDPUI7WQrpMO5PnkVeV9XJyw+OgS1SWCJT1LrZZuWPosPzj/12XoZexlXQWGBasLBpYzltesgFakrehaabyycOXQqpBVdauJq7NW/77Gfk3pmndrE9d2FuoVriocXBeyrrFIqUhUdGe91/o9P6F/4v/Uu8Fpw84N34o5xVdK7EvKS75sZG+88rPDzxU/yzelb+rd7Lp59xbsFsGW21t9t9aVqpbmlw5ui9zWVkYvKy57t33h9svlzuV7dhB3SHfIKiIqOnaa7Nyy80slr/JWlX9Vc7Vu9YbqD7s4u/p3++1u2qO3p2TP51/4v9ytCalpqzWrLd+L3Zu39+m+hH3dv7r/Wr9fe3/J/q8HBAdkdTF15+vd6usbdBs2N8KN0saRg/MOXj8UeKijybapplmjueQwOCw9/PxI6pHbLeEtXUfdjzYdMz1W3UptLW6D2pa2jbbz2mUdyR19x8OOd3V6dbb+ZvfbgROGJ6pOqp/cfIp4qvCU/HT+6bEzwjMvz2acHexa2PXgXNK5m+fnnO+9EH7h0sXgi+e6Gd2nL3lfOnHZ8/LxK+5X2q+6Xm3rcelp/d3l99Ze1962a27XOq57XO/sm9V3qt+3/+yNwBsXbzJvXr01+1bf7fjbd+/MuyO7y7k7fC/73uv7effHH6x6iHlY/EjlUflj3ce1f1j+0SxzlZ0cCBzoeRL75MEge/DFn+I/vwwVPqU8LX9m8Kx+2HH4xEjwyPXnc58PvRC+GH9Z9JfqX9WvLF4d+9vv757RpNGh16LX8jcb32q9PfDO+V3XWPTY4/c578c/FH/U+lj3yf1T9+fEz8/GF3/Bfan4avm181v4t4fyHLlcyBKxvrcCKMTh9HQA3hwAgJIMABXpiYlzJ3vo7wZN9v3fCfwvnuyzv5srAE3IMNEKMVYB0OKHtLCIKyEehXicH4CdnBT+bxOnOzlOaik1AoAzlMvf5AJAQPxLiFw+Hi2Xf61Gir0JwKnhyd59wrDIH00T1SFTh9f/TVID/sv+BYmREGAWHocYAAAABHNCSVQICAgIfAhkiAAAAF96VFh0UmF3IHByb2ZpbGUgdHlwZSBBUFAxAAAImeNKT81LLcpMVigoyk/LzEnlUgADYxMuE0sTS6NEAwMDCwMIMDQwMDYEkkZAtjlUKNEABZgamFmaGZsZmgMxiM8FAEi2FMk61EMyAAAA2klEQVRIiWM0s7T7z8DAwCAiIvzH1Nhoi6SEaHVGRsY1BhygvqF5/emz53zevHnLwsDAwMBgZmn3PyUt+wUuDbhASlr2CzNLu/8MXr6Bv0nVDANevoG/mUyNjbaQa4CpsdEWJkkJ0WpyDaBEL/UAI7rA/////+PVwMiIooeJ2i4aADBjxgwtSvQyPX/xupVcA56/eN3KdPrsOR9yDTh99pwP05s3b1nIzUxv3rxlYaQ0O1OeDoZ5efAfCnDJEywPCBnAwECFWBjk5QFdwoCJpuUBIxTgkh8m5QEA4ESeKl1VgwMAAAAASUVORK5CYII=');
Copy link
Contributor

Choose a reason for hiding this comment

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

image

can we use single toggle icons?

@fabpot
Copy link
Member

fabpot commented Apr 6, 2019

Is this one ready?

@dFayet
Copy link
Author

dFayet commented Apr 6, 2019

@stof I've added more contrast for the red color, not only for the exception title, but for all red text (ref updated pictures)

@fabpot fabpot force-pushed the feature/web-profiler/fix-dark-theme branch from 2aa3a5e to 8a9c2c8 Compare April 6, 2019 18:06
@fabpot
Copy link
Member

fabpot commented Apr 6, 2019

Thank you @dFayet.

@fabpot fabpot merged commit 8a9c2c8 into symfony:4.2 Apr 6, 2019
fabpot added a commit that referenced this pull request Apr 6, 2019
This PR was squashed before being merged into the 4.2 branch (closes #30860).

Discussion
----------

[Profiler] Fix dark theme elements color

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | yes
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #29194   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | -

I think the color handling of the code highlighted (exceptions, ...) is quite questionnable, but as we use the `highlight_file()` function, I don't really see an other way to handle it, tell me what you think.

###  Before/After
**Form**
![Capture d’écran de 2019-04-04 10-07-18](https://user-images.githubusercontent.com/7721219/55539945-2cfe0580-56c2-11e9-8e5c-a4b6352ab12d.png)

![Capture d’écran de 2019-04-04 10-03-27](https://user-images.githubusercontent.com/7721219/55539906-15268180-56c2-11e9-90e7-326cc1c01ccd.png)

**Twig render graph**
![Capture d’écran de 2019-04-04 10-07-36](https://user-images.githubusercontent.com/7721219/55539969-3b4c2180-56c2-11e9-9064-1fd10bf1da02.png)

![Capture d’écran de 2019-04-06 19-37-02](https://user-images.githubusercontent.com/7721219/55673094-693c8c00-58a3-11e9-90a4-76f6b312d516.png)

**Exceptions**
![Capture d’écran de 2019-04-04 10-08-19](https://user-images.githubusercontent.com/7721219/55540012-51f27880-56c2-11e9-930b-98d680093e49.png)

![Capture d’écran de 2019-04-06 09-39-27](https://user-images.githubusercontent.com/7721219/55667421-4f2b8b00-585c-11e9-81c7-dc09bd982375.png)

**Doctrine queries**
![Capture d’écran de 2019-04-04 10-08-37](https://user-images.githubusercontent.com/7721219/55540059-73536480-56c2-11e9-9b0d-ea691a16a1a5.png)

![Capture d’écran de 2019-04-04 10-04-47](https://user-images.githubusercontent.com/7721219/55540068-78b0af00-56c2-11e9-88f7-2181800006a7.png)

Commits
-------

8a9c2c8 [Profiler] Fix dark theme elements color
@dFayet dFayet deleted the feature/web-profiler/fix-dark-theme branch April 6, 2019 19:35
fabpot added a commit that referenced this pull request Apr 8, 2019
This PR was merged into the 4.2 branch.

Discussion
----------

Fix dark themed componnents

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes/no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #...   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

Small follow up of #30860

![image](https://user-images.githubusercontent.com/1047696/55705391-f5070300-59de-11e9-8d7e-195ea74d17c3.png)

see #29194 (comment)
___

![image](https://user-images.githubusercontent.com/1047696/55705263-b40eee80-59de-11e9-9503-ba4adf64857c.png)

see #29194 (comment)

Commits
-------

d2f2e56 Fix dark themed componnents
@fabpot fabpot mentioned this pull request Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants