search

bitcoin-core / gui

Bitcoin Core GUI staging repository
https://github.com/bitcoin/bitcoin
MIT License
604 stars 266 forks source link

wallet: Improve error log color in the console #823

Closed ChillerDragon closed 3 months ago

ChillerDragon commented 5 months ago

before

image

after

image

DrahtBot commented 5 months ago

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

ChillerDragon commented 5 months ago

A light theme indeed is smoother on the eye

image

I still think the softer color is better. And it fits both dark and light themes.

katesalazar commented 5 months ago

~

luke-jr commented 5 months ago

Pink on white probably isn't a good idea...

hebasto commented 4 months ago

cc @GBKS for a designer's opinion.

I still think the softer color is better.

I'm not a designer, but I find the current implementation the most appealing.

As for implementation, a named constant should be used in the code.

GBKS commented 4 months ago

cc @GBKS for a designer's opinion.

Personally, I find the new color more pleasant, and it goes better with the teal. It's a bit too toned down for me, and the old red does a better job at drawing attention and ensuring you look at it, which is appropriate for a scammer warning. I'd probably look at something in-between (strong red, but not jarring), and also tweak the other colors used in the logs so they all go together well and have good contrast on light and dark backgrounds (the teal has weak contrast on black right now). YMMV.

Just generally, a good practice for design systems is that you use the same color palette throughout the application (e.g. same red(s) everywhere), and fine-tune the colors so they go well with each other (-> you work on the whole system). For light/dark modes, it's also good to make tweaks (e.g. slightly different red in dark mode than in light mode), since our eyes pick up colors differently based on environment/lighting/.... I don't know if one of the ambitions is to have a super tight design system throughout the application, so this practice may or may not be relevant.

Not feeling super strong either way about this PR, feel free to ignore my input.

hebasto commented 3 months ago

This PR touches stuff that requires attention from both design and coding perspectives.

As for designers, they are currently focused on the future Bitcoin Core App design, which is being implemented in https://github.com/bitcoin-core/gui-qml.

From a coding perspective, I believe that introducing "magic" numbers spread across the code is an inferior implementation compared to using theme-specific palettes or the currently used named constants.

Therefore, I'm going to close this PR.