-
-
Notifications
You must be signed in to change notification settings - Fork 564
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
Less color for the Query Log #1872
Conversation
Signed-off-by: DL6ER <[email protected]>
Signed-off-by: DL6ER <[email protected]>
I didn't test it but I like it :) Do you have a screenshot with the default light theme? |
LGTM even though the yellow color has low contrast but it's unrelated to this patch :) |
This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there: https://discourse.pi-hole.net/t/pi-hole-ftl-v5-9-web-v5-6-and-core-v5-4-released/49544/1 |
Thanks for the suggestion. We'll check if it is even possible to automatically colorizing the domain (I don't think the time but type needs colorization) depending on your screen size. If not, I'll add a device-dependent toggle option (so it can be adjusted on every device/browser individually). What do you think? |
I think you made an issue where there were none Now there is a issue it makes it harder to notice blocked queries, The whole page looks like a block of text. |
Changes sometimes have unintended side-effects. Not a good justification to never change anything. Nobody realized this in our beta testing and I've seen comments that the query log is now easier to read - these users may all have used larger screens. I was asking what users are thinking about the proposed solution. As you might have seen I've agreed that something will happen. I still want to hear your thoughts about what should happen. Pi-hole in a community-driven project after all, hence, your comments are appreciated. |
I only looked at it briefly when it was initially made available and agreed on the fact that it makes the page look less 'busy'. Hence, a simple toggle / checkbox would be useful so both options would be possible (not very sure if this should be automated based on screen size to be honest) |
#1888 adds a setting to toggle the pre-v5.4 behavior (colorize entire rows). As this setting will be stored per-device you can decide to have full colored rows, e.g., on mobile and colored status only on desktop computers. Or colors everywhere or nowhere. Just as you like. |
The thing we have to be very careful of is red/green as the sole indicator. Red/Green colorblindness is a major accessibility issue. |
It's been Red/Green for a very long time. |
That doesn't make it any easier for accessibility. |
Add a flag, tick/Green - exclamation mark/Red |
@HenrysCat I've been playing with this thought. The question is also where to put it. Should it have its own column? And moving the query type further to the back, as suggested by @dschaper, makes sense, too. |
Or the word "OK" in Green and "Blocked" in Red. Which is exactly what I proposed. |
changing the colorization to just two letters such a status of "OK" instead of the whole line (which is easier to see) should now at least also present the option for us to toggle colorizing the entire row or just the status |
Another thought - are all queries either allowed or blocked? If so, only applying a red color to the blocked items may make things cleaner. If it’s not red, then it wasn’t blocked. |
Please, i want more colours back :-( |
@Lycidias93 #1893 is already merged and will be part of the next Pi-hole release. |
By submitting this pull request, I confirm the following:
git rebase
)What does this PR aim to accomplish?:
Make the Query Log less colorful by, instead of coloring the entire rows, emphasize only the main part.
This is currently lost in the colorful noise IMO.
I've also tried coloring the domain as well, but this looks somehow odd to me.
How does this PR accomplish the above?:
Do not colorize the entire row but only the status.
What documentation changes (if any) are needed to support this PR?:
None