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

More Readable UI #10

Merged
merged 5 commits into from
Jun 27, 2020
Merged

More Readable UI #10

merged 5 commits into from
Jun 27, 2020

Conversation

lyokum
Copy link
Contributor

@lyokum lyokum commented May 22, 2020

Changes:

  • moved the battery percentage from the gauge to the title of the
    section
  • bold all section titles
  • surround section titles with "|" symbols
  • bold and underline subsection titles (removed dark grey color)

Changes:
- moved the battery percentage from the gauge to the title of the
  section
- bold all section titles
- surround section titles with "|" symbols
- bold and underline subsection titles (removed dark grey color)
@svartalf
Copy link
Owner

Thank you for this contribution!

For the reference, screenshots for current master HEAD 17d7edc and
8e580d8 from this PR:

before
before

after
after

moved the battery percentage from the gauge to the title of the section

It is an interesting approach with moving state of charge directly into the header itself.
I would say that we can go with this solution, but it got me wondering if it is possible to make text and background colors for the text in this gauge inversed? Let's check that first.

bold all section titles
surround section titles with "|" symbols

I'm sorry, but I do not like this change, it adds too much visual noise and distracts from the main content; I'm strongly against merging it. One possible solution is too add spaces before and after the block label, that should help with readability.

bold and underline subsection titles (removed dark grey color)

Same to the previous change, bold and underline are too noisy, but removing dark grey color looks better.

To summarize, I tried to draw what can be changed to make it look both clean and also readable:

idea

Let me know what are you thinking about it.

Changes:
- moved percentage label to separate block
- remove title bolding & surround titles with spaces
- remove subtitle bolding (still underlined)
- "State of charge" -> "Charge Percentage"
@lyokum
Copy link
Contributor Author

lyokum commented May 22, 2020

Thanks for your detailed and helpful feedback!

I agree with your comments about overcluttering the UI. I have added the more subtle spaces around the titles and removed the bolding of the titles. I also removed the bolding on the subsection titles (although I kept the underlines to distinguish each subsection title from the data points).

Regarding the inverted colors for the charge status, I looked into adding that feature. However, it seems that the Gauge widget from tui-rs doesn't provide an interface to control the text background separately from the colors of the Gauge as far as I can tell. Thus, we would need to either write a wrapper around the Gauge widget or write a custom widget to implement this (doable, but might be overkill for this component).

An alternative solution for making the percentage more readable is to separate it into its own Block next to the Gauge. You can view this change in the screenshot below (along with the other fixes):

battop-more-readable-2

Let me know what you think about this design. Along the way, I also thought that "Charge Percentage" might be a better name than "State of charge" for that section. If you don't like that name, I can revert it back to "State of charge."

@svartalf
Copy link
Owner

I agree with your comments about overcluttering the UI. I have added the more subtle spaces around the titles and removed the bolding of the titles. I also removed the bolding on the subsection titles (although I kept the underlines to distinguish each subsection title from the data points).

You are right that subsections should be separated from each other somehow; dark grey color for these titles were adding enough visual space to do that, but now it is problematic.
Maybe we should just replace underlining with bold font? I wonder if it will be more readable.

Also, these newly appeared colons should be removed too, table columns are short enough, so it is easy to match keys with the corresponding values and colons are not adding any value at all.

Thus, we would need to either write a wrapper around the Gauge widget or write a custom widget to implement this (doable, but might be overkill for this component).

Yeah, it is totally not worth the fuzz, thanks for checking that out.

An alternative solution for making the percentage more readable is to separate it into its own Block next to the Gauge. You can view this change in the screenshot below (along with the other fixes):

That looks nice! It also reminds me how htop displays its gauges:

htop

I like the idea that the gauge and its text do not have any border between them, they look semantically tied together. Since tui "allows" to collapse borders of two blocks (basically by removing the adjoined borders), maybe it is worth to check how would it look.
Not sure if there should be a whitespace char between gauge and text or not, maybe both options should be explored?

Also note how text color is changed based on its value — "yellow" and "red" colors are used to attract the users' attention, but if everything is okay it is rendered in grey color.
That looks great and we should totally "steal" that idea :)

Let me know what you think about this design. Along the way, I also thought that "Charge Percentage" might be a better name than "State of charge" for that section. If you don't like that name, I can revert it back to "State of charge."

"State of charge" is an industry standard, I prefer to leave it as is. While the title might be confusing from a first glance (especially if you are not familiar with the correct terms), gauge and its value are explaining what is represented in there.

Changes:
- removed block borders between gauge and percentage
- added separator character between gauge and percentage
- color of the percentage changes if battery low
- removed colons from subsection keys
- changed subsection titles to be bolded (no underline)
- "Charge Percentage" -> "State of charge"
@lyokum
Copy link
Contributor Author

lyokum commented May 26, 2020

battop-more-readable-3-full
battop-more-readable-3-low
Note: For the second picture, I manually set the value to show the color change.

Okay, I have removed the colons and reverted the "Charge Percentage" change. I also changed the subsection titles to be bolded.

For the battery gauge, I removed the block borders between the gauge and the percentage text. However, I found that without some sort of ending separator, it was hard to visually read how full the gauge was. Thus, I added a unicode box drawing separator to depict the end of the gauge. I also set the text to match the gauge color except when the gauge color is green.

As always, let me know what you think :D

@lyokum
Copy link
Contributor Author

lyokum commented May 26, 2020

For reference, here are some comparison images with whitespace as a separator and without any separator.

With whitespace separator:
battop-more-readable-4-whitespace-full
battop-more-readable-4-whitespace-low

With no separator:
battop-more-readable-4-none-full
battop-more-readable-4-none-low

@svartalf
Copy link
Owner

I love that!

Box drawing character looks kinda foreign, but the whitespace separator suits here amazingly.

I'm still not sure what would be the best option about making gauge text gray: it looks organic in htop because gauge itself is filled from the left and there is a huge gap between it and its value, so it is not distracting at all. In our case it goes from high to low point, so I'm still controversial about it.

pngout

To be honest, first option (with both gauge and text having the same color all the time) looks more solid now, so my first idea about making them behave differently was wrong. What is your opinion on that?

@svartalf svartalf merged commit 4785f7d into svartalf:master Jun 27, 2020
@svartalf
Copy link
Owner

Since this PR got kinda stale, I decided to merge it after all since it contains a lot of UI improvements.

Thank you @lyokum once again for making it!

svartalf added a commit that referenced this pull request Jun 27, 2020
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.

2 participants