simplesamlphp / SAML-tracer

Browser extension for examining SAML messages
https://addons.mozilla.org/nl/firefox/addon/saml-tracer/
BSD 2-Clause "Simplified" License
141 stars 39 forks source link

Made compatible with “Dark mode” setting from the browser #85

Closed doedje closed 11 months ago

doedje commented 11 months ago

Use of variables on :root in samltrace.css for all the colors and button-icons, which will be overwritten in dark-mode, if needed.

I tried to stay as much to the original as possible for the 'light mode'. But I did bring down the number of different shades of grey that were in use. Notable changes are: a) the model is little bit more subtle and b) the error-message in the import dialog I changed to something that works equally fine on both white and black backgrounds.

tvdijen commented 11 months ago

I would also be in favor of a little-less-than-black! That import-dialog could use some more TLC though.. The [Import] button is really hard to read because the rest of the dialog is so black..

doedje commented 11 months ago

Thanx for the feedback, I totally agree on the little-less-than-black! I'll colorpick the colors from Firefox's Web developer tools, for both dark and light mode. (the import button in the screenshot is indeed hard to read, but it is in its disabled state in the screenshot, but I'll look into that. Are you okay with what I did to the modal?

khlr commented 11 months ago

Are you okay with what I did to the modal?

I'd prefer the rose/light red background for the error messages instead of pure red. But that's details 😉

doedje commented 11 months ago

I pushed a new commit in which I selected all colors from the Firefox Web Developer Tool, both dark and light mode. Same goes for the error message in the import pane. I also took the liberty of changing the font-family for #request-info-content, in my case it now uses the same font as in the Firefox Web Developer Tool, resulting in much better readable info.

doedje commented 11 months ago

Please dont merge yet, I am also color picking the colors for the syntax highlighting from the devtools. Will finish that tomorrow and do another push

doedje commented 11 months ago

This should be it, for now! =]

khlr commented 11 months ago

Nice, I really like it 😊

Just one thing, that you could adjust, if you like to: Requests without responses have the default color (#d7d7db) which appears very bright (almost white against the dark background). Given that these requests are the least important most of the times, I'd prefer to darken them. Something like the default text color for the bright theme (#737373) seems quite nice to me. What do you think?

doedje commented 11 months ago

I changed it as per your suggestion. But shouldn't the "Filter resources" button take care of all those uninteresting request in the first place? I have the feeling this filtering was 'better' in the past and it is now (last couple of years) showing me much more uninteresting request than it used to.

khlr commented 11 months ago

[...] But shouldn't the "Filter resources" button take care of all those uninteresting request in the first place?

Those responseless requests and requests for resources are two different things. The latter are usually always not of interest - hence the filter-away-feature. Responeless requests, however, can actually be very interesting! Take a look at my last screenshot - the POST to sp.example.com. There was no response, because there's nothing at all. But in a real situation this could be a valid thing. Consider the service at sp.example.com by mistake just isn't running. You probably would the request to appear in the list anyway.

I have the feeling this filtering was 'better' in the past and it is now (last couple of years) showing me much more uninteresting request than it used to.

There are quite a lot of reqeusts to resources that don't receive a response. So it's a combination of both. Why does this happen? I don't know. Maybe related to caching? If the filter-resources-feature was a bit smarter it probably detected those responseless resource requests anyway. Maybe something to enhance...

khlr commented 11 months ago

Merged 😊 Thank you!