Project

General

Profile

Actions

Feature #13520

closed

Improve Thermal Sensors Dashboard widget readability

Added by GChuf 6 about 2 years ago. Updated 10 days ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Dashboard
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
24.11
Release Notes:
Default

Description

https://github.com/pfsense/pfsense/pull/4616

The PR features a couple of changes.

1. improved upon the graph layout - changed the title to be above the progress bar and added a margin on bottom
Makes it easier to amke it out which bar belongs to which sensor.

2. Fixed a bug which would always build the rows in html instead of only updating them.
If you have a better solution than mine, please let me know.

3. "fixed" dark pfsense theme so that progress-bar matches the dark green color. Did not change other themes.

4. The second commit makes sure the widgets get updated as soon as possible after the user refreshes the page.
Without this, user would wait a couple of seconds before getting thermal sensors and other widgers' data.

A couple of before/after pictures regarding the first commit:

before

after


Files

before.png (16.6 KB) before.png before GChuf 6, 09/26/2022 03:28 AM
after.png (70.1 KB) after.png after GChuf 6, 09/26/2022 03:28 AM
Actions #1

Updated by Marcos M 2 months ago

  • Subject changed from Update thermal sensor widget to Improve thermal widget readability
  • Status changed from Pull Request Review to Feedback
  • Target version set to 2.8.0
  • Plus Target Version set to 24.08
The feature request has been (partly) implemented:
  • 4b0deb88ca437dea96d719388a4b9136b49123ee: This change affects all progress bars, not only the thermal widget. Instead of overriding the default theme with `!important`, use the same selector and leave the override behavior to the include order.
  • b0c455aa5d3f1c3b1926dbcca0eacbe4f57e3ef6: Improve the thermal widget readability by adding a margin and sorting. The label is kept below the progress bar to match the rest of the GUI (e.g. the disk widget).

Other changes to be considered separately.

Actions #2

Updated by GChuf 6 2 months ago

First of all thank you for looking at the PR and your commits. I agree with all the changes you've made to my commits, I think it's better this way.

Regarding the rebuilding -

While looking at my commit it occurs to me that I probably didn't mention a very important detail: the variable firstTime doesn't work correctly and the buildThermalSensorsDataGraph function is the one that's executed on every ajax call. updateThermalSensorsDataGraph never gets executed. This generates unnecessary load on clients and slows down the refresh.
Maybe it would be better to fix the issue with the variable first. This issue is present in some other widgets as well. I can't remember all the details since the PR is almost 2 years old.

I'd like to contribute more to the widget refresh system/widgets code, however it's hard to track things that are as old as this. I have some other commits written, but I'd like to ask if it's possible for you to review the code a bit sooner (a couple of weeks, a month). I know you're not obligated to do anything here, but I'd like to know if my commits will make it into the project and ultimately, if it's worth my time as well.

For example, I have a simple fix for the widget refresh system as I think the "refresh" doesn't work as intended at the moment (refresh interval is bugged, and widgets don't all update at the same time). I'll take a look at the code, clean it up and submit a PR. Maybe the No timeout on first load commit can go there as well.

Let me know what you think of the html rebuilding and the firstTime variable.
Thank you!

Actions #3

Updated by Marcos M 2 months ago

I agree it's slow. My only advice is to keep the commits focused; the more detail/explanation about why a change is made the better.

Yes, I will look at it sooner - feel free to tag me in the PR.

Actions #4

Updated by GChuf 6 2 months ago

Thank you - I've created another PR for the widget refresh, and I'll put optimizations in yet another PR.
You can close this redmine issue now.

Actions #5

Updated by Jim Pingle about 2 months ago

  • Subject changed from Improve thermal widget readability to Improve Thermal Sensors Dashboard widget readability
  • Category changed from Web Interface to Dashboard
Actions #6

Updated by Jim Pingle about 1 month ago

  • Plus Target Version changed from 24.08 to 24.11
Actions #7

Updated by Jim Pingle 10 days ago

  • Status changed from Feedback to Closed
  • Assignee set to Marcos M
  • % Done changed from 0 to 100

The parts that were implemented appear to be OK, other parts can be moved to separate issues as needed if they are still desirable changes.

Actions

Also available in: Atom PDF