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

fix contrast in console ngql #771

Closed

Conversation

johnny-smitherson
Copy link

#770

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

#770

Description:

Very bad contrast on GQL query shown in studio console #770

How do you solve it?

color: #111;

Special notes for your reviewer, ex. impact of this fix, design document, etc:

@CLAassistant
Copy link

CLAassistant commented Feb 29, 2024

CLA assistant check
All committers have signed the CLA.

@johnny-smitherson
Copy link
Author

before
Captur2e
after
aaaaaaCapture

@QingZ11
Copy link

QingZ11 commented Mar 4, 2024

@johnny-smitherson Thank you for your contribution 🤗

@yyh0808
Copy link
Contributor

yyh0808 commented Mar 4, 2024

Great PR!
I suggest changing the color value to #0B8373 to align with our design standards so we can merge your PR. Additionally, I recommend using the Roboto Mono font to comply with our typography guidelines.
截屏2024-03-04 17 11 38

@johnny-smitherson
Copy link
Author

changed font and colors

Comment on lines 26 to 29
font-family: Roboto-Regular, sans-serif;
font-family: Consolas, "Courier New", monospace;
font-weight: 500;
font-size: 18px;
color: #00bfa5;
color: #111;
Copy link
Contributor

Choose a reason for hiding this comment

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

designer's suggested style

font-family: Roboto-Mono, sans-serif;
font-weight: 500;
font-size: 16px;
color: #0B8373;

image

image

@@ -23,10 +23,10 @@
border-radius: 6px;
padding: 11px 14px;
cursor: pointer;
font-family: Roboto-Regular, sans-serif;
font-family: Roboto-mono, monospace;
font-weight: 500;
font-size: 18px;
Copy link
Contributor

Choose a reason for hiding this comment

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

adjusting the font-size to 16px might look better ?

@johnny-smitherson
Copy link
Author

johnny-smitherson commented Mar 5, 2024

Thank you for your suggestions and insight into how this problem came to be.

I still am not convinced by your Green-on-Green, Red-on-Red and "smaller font" design guidelines.

I don't think developers that look at this exact field many hours a day for small bugs in the code prefer pretty-looking UI to easy-readable UI - maybe this one can be uglier than normal?


After reading the pixel color values from the images above:

1_Capture

contrast 3.5 = "Poor"

1111111Capture

It's my personal opinion that what we're ending up with isn't that much different from where we left.

with black foreground: contrast 16 = "Super"

222222222Capture

Maybe I'm looking too much at this page?

Maybe the solution would be to go further, e.g. Use same syntax highlighting from the code editor, on very slightly gray bg?.

I will probably continue to use my personal fork that uses black text foreground instead.

Please feel free to take over the PR or close it and hand it off to the product design.

@QingZ11
Copy link

QingZ11 commented Mar 6, 2024

@johnny-smitherson As an individual, I agree with your viewpoint, and I also do believe that readability is more important than aesthetic design. Your PR will be handed over to the UI designer @yyh0808 of NebulaGraph for handling.

Once again, thank you, Johnny, for your feedback. Both the PR and UI feedback are beneficial to us.

@huaxiabuluo
Copy link
Contributor

Thank you for your suggestions and insight into how this problem came to be.

I still am not convinced by your Green-on-Green, Red-on-Red and "smaller font" design guidelines.

I don't think developers that look at this exact field many hours a day for small bugs in the code prefer pretty-looking UI to easy-readable UI - maybe this one can be uglier than normal?

After reading the pixel color values from the images above:

1_Capture [contrast 3.5 = "Poor"](https://coolors.co/contrast-checker/0b8373-b2ece4) 1111111Capture It's my personal opinion that what we're ending up with isn't that much different from where we left.

with black foreground: contrast 16 = "Super"

222222222Capture Maybe I'm looking too much at this page?

Maybe the solution would be to go further, e.g. Use same syntax highlighting from the code editor, on very slightly gray bg?.

I will probably continue to use my personal fork that uses black text foreground instead.

Please feel free to take over the PR or close it and hand it off to the product design.

hi @johnny-smitherson, thank you for your feedback.

In popular component libraries, there are several color combinations for successful message displays:

  1. green background + black text (AntD <Alert type="success" >)

image

  1. light green background + dark green text (MUI <Alert severity="success">)

image

  1. light green background + dark green text (AntD <Tag color="success">)

image

  1. light green background + white text (MUI <Alert variant="filled" severity="success">)

image

  1. others

https://chakra-ui.com/docs/components/alert#status
image

https://element-plus.org/en-US/component/alert.html
image

https://react-bootstrap.netlify.app/docs/components/alerts#examples
image

https://tailwindui.com/components/application-ui/feedback/alerts#component-aa7cc38968c95d870db6ba62e76b8e0f
image

Each combination has its own use scenarios, but in general, there may not be such strict rules. Studio 3.x version does not provide the function of customizing themes, but in the new version, we provide the ability for users to match colors according to their own preferences, please stay tuned.

@huaxiabuluo huaxiabuluo closed this Mar 6, 2024
@johnny-smitherson
Copy link
Author

johnny-smitherson commented Mar 6, 2024

Thank you for the examples above, I see where the problem comes from.

All examples you sent are illustrated as alerts - toasts, messages, responses, things like "Error 404" or "Operation Success" where the color of the message is 90% of the message information.

The field we're talking about is different - it's ngSQL code the programmer put in 30 mins ago, that needs to be easily readable.

I think the problem we're facing is caused by the design choice of combining the operation status (success or fail) with the query content.

Maybe the fix here is to show a green SUCCESS or red ERROR box, with whatever contrast looks pretty, but the source code of the query should be very high contrast.

Here is what i mean, courtesy of mspaint.exe

309706445-d814fad9-d98e-4106-b0d5-5a47dcb6ddf3 309706714-6d51e187-edeb-457d-8785-e6ea7f947d46

As you can see, it does not matter how pretty the "Success" box is, I will still be able to read the query. I can also quickly pattern match it to be code, not random text, because of the syntax highlighting and font being the same as the query box. The code also looks the same way regardless if it was in success or error.

Good luck!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants