Change address / amount error background

[w0xlt]

This PR proposes a small change in QLineEdit when there is an error in the input.

master

PR

This also shows good results when combined with other open PRs.

#537

#533

[DrahtBot]

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #537 (Point out position of invalid characters in Bech32 addresses by luke-jr)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[luke-jr]

This also shows good results when combined with other open PRs.

#537

Well, now the background would be theme-dependent, and #537 will need to figure out a reasonable contrasting colour...

[promag]

IMO this improves text contrast.

However, other improvements could be made as well, some suggestions:

• consistent error message near the invalid fields, for instance, the error message for invalid change address is displayed on the right while the upcoming bech32 error shows on the top
• the amount field only turns red when clicking send and it's zero
• the send button remains enabled while a field is invalid, maybe it should be disabled

[w0xlt]

consistent error message near the invalid fields, for instance, the error message for invalid change address is displayed on the right while the upcoming bech32 error shows on the top

@promag I addressed this suggestion in https://github.com/bitcoin-core/gui/pull/560

[RandyMcMillan]

@jarolrod - thanks for pinging me on this - my apologizes for not seeing it sooner...

@w0xlt,

Just to define the issue with more clarity,

We can see using this color contrast calculator that in the light theme (on Mac) that the black text on the rgb(255,128,128) is "ok" kinda... :)

But when we switch to the dark theme (on Mac) the contrast becomes unsatisfactory from a contrast perspective and is apparent just by looking at it.

[RandyMcMillan]

When fe7c81e34e2e16c4a5ec967645ebb49e161d3a25 is tested against light/dark theme changes (on Mac) we get...

which doesn't work because the text color is changed based on system theme...

[RandyMcMillan]

[RandyMcMillan]

Here are a couple patches to experiment with palleteRole in Qt: You will have to call the text field to update during the system theme change - other wise it will only update when it receives focus.

diff --git a/src/qt/guiconstants.h b/src/qt/guiconstants.h
index 60e8ee2420..8b37c5dfb4 100644
--- a/src/qt/guiconstants.h
+++ b/src/qt/guiconstants.h
@@ -25,7 +25,7 @@ static const int STATUSBAR_ICONSIZE = 16;
 static const bool DEFAULT_SPLASHSCREEN = true;
 
 /* Invalid field background style */
-#define STYLE_INVALID "border: 3px solid #FF8080"
+#define STYLE_INVALID "color: palette(highlighted-text); background-color: pallet(shadow); border: 3px solid #FF8080"
 
 /* Transaction list -- unconfirmed transaction */
 #define COLOR_UNCONFIRMED QColor(128, 128, 128)

diff --git a/src/qt/guiconstants.h b/src/qt/guiconstants.h
index 60e8ee2420..dca896ca7e 100644
--- a/src/qt/guiconstants.h
+++ b/src/qt/guiconstants.h
@@ -25,7 +25,7 @@ static const int STATUSBAR_ICONSIZE = 16;
 static const bool DEFAULT_SPLASHSCREEN = true;
 
 /* Invalid field background style */
-#define STYLE_INVALID "border: 3px solid #FF8080"
+#define STYLE_INVALID "color: palette(highlighted-text); background-color: pallet(window); border: 3px solid #FF8080"
 
 /* Transaction list -- unconfirmed transaction */
 #define COLOR_UNCONFIRMED QColor(128, 128, 128)

[w0xlt]

@RandyMcMillan Thanks for the detailed review and suggestions. I will apply and test them.

[w0xlt]

@RandyMcMillan I tried your suggestions (palette(window), palette(shadow)) and also palette(base), but none of them worked. However I didn't test on MacOS, I did it on Ubuntu by setting the theme using Qt5 Settings.

[Rspigler]

I actually prefer the red highlighting. IMO it's more contrasting and more of an alert