-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Closes #8232: Add color show full 100% utilization #8816
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
Closes #8232: Add color show full 100% utilization #8816
Conversation
| {% endif %} | ||
| </div> | ||
| {% endif %} | ||
| {% endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a newline
| aria-valuenow="{{ utilization }}" | ||
| {% if is_full_danger %} | ||
| class="progress-bar" | ||
| style="width: {{ utilization }}%; background-color: #800080;" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any demo image for this? Including all four colors in light mode and dark mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I choose constants color for 100% utilization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| is_full_danger = False | ||
| if utilization == 100: | ||
| is_full_danger = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic can be cleaned up considerably. There should be no need to store a variable indicating whether the utilization value is 100: we can just reference utilization directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry, I misunderstand something
| utilization = int(float(child_ips.size) / prefix_size * 100) | ||
| utilization = float(child_ips.size) / prefix_size * 100 | ||
|
|
||
| return min(utilization, 100) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The call to min() is needed to address instances where utilization is reported to be greater than 100%.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I forgot that context
| utilization = int(float(child_prefixes.size) / self.prefix.size * 100) | ||
| utilization = float(child_prefixes.size) / self.prefix.size * 100 | ||
|
|
||
| return min(utilization, 100) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See below
| aria-valuenow="{{ utilization }}" | ||
| {% if utilization == 100 %} | ||
| class="progress-bar" | ||
| style="width: {{ utilization }}%; background-color: #800080;" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bar color needs to be changed by modifying the bar_class context var inside the utilization_graph template tag. (bg-secondary seems like a good choice.) Please avoid hard-coding color values directly in the HTML.
|
Thanks for your work on this @tranthang2404! |


Fixes: #8232