frontend: components: health: HealthTrayMenu: Change disk warning icon to open disk page#3786
Open
patrickelectric wants to merge 1 commit intobluerobotics:masterfrom
Open
frontend: components: health: HealthTrayMenu: Change disk warning icon to open disk page#3786patrickelectric wants to merge 1 commit intobluerobotics:masterfrom
patrickelectric wants to merge 1 commit intobluerobotics:masterfrom
Conversation
…n to open disk page Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors the disk usage warning in the HealthTrayMenu from a details popover to a simple clickable warning icon that routes directly to the disk tools page, and removes now-unused computed helpers for formatted disk sizes. Sequence diagram for updated disk warning icon navigationsequenceDiagram
actor User
participant HealthTrayMenu
participant Router
participant DiskToolsPage
User->>HealthTrayMenu: Click disk_warning_icon
HealthTrayMenu->>Router: push(/tools/disk)
Router-->>DiskToolsPage: Render DiskToolsPage
DiskToolsPage-->>User: Display disk tools and usage details
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
patrickelectric
requested review from
Williangalvani,
joaoantoniocardoso,
joaomariolago and
nicoschmdt
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The hardcoded route string in
@click="$router.push('/tools/disk')"could be replaced with a named route or shared constant to avoid breakage if the path changes. - The disk usage warning message template is now duplicated between the
v-ifandv-tooltipusages; consider extracting it into a computed property for easier maintenance and consistency.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The hardcoded route string in `@click="$router.push('/tools/disk')"` could be replaced with a named route or shared constant to avoid breakage if the path changes.
- The disk usage warning message template is now duplicated between the `v-if` and `v-tooltip` usages; consider extracting it into a computed property for easier maintenance and consistency.
## Individual Comments
### Comment 1
<location> `core/frontend/src/components/health/HealthTrayMenu.vue:3-12` </location>
<code_context>
- :close-on-content-click="false"
- nudge-left="150"
- nudge-bottom="25"
+ <v-icon
+ v-if="disk_usage_high"
+ v-tooltip="`High disk usage! (${disk_usage_percent.toFixed(1)}%) please clean up the disk`"
+ class="px-1 blinking white-shadow"
+ color="red"
+ @click="$router.push('/tools/disk')"
>
- <template
- #activator="{ on, attrs }"
- >
- <v-card
- elevation="0"
- color="transparent"
- v-bind="attrs"
- v-on="on"
- >
- <v-icon
- v-if="disk_usage_high"
- class="px-1 blinking white-shadow"
- color="red"
- :title="`High disk usage high! (${disk_usage_percent.toFixed(1)}%) please clean up the disk`"
- >
- mdi-content-save-alert
- </v-icon>
- </v-card>
</code_context>
<issue_to_address>
**issue (bug_risk):** Clickable icon should be keyboard-accessible and have an explicit accessible label.
As this icon is now the sole way to reach the disk tools page, it needs to be fully accessible. A plain `<v-icon>` with only a click handler likely isn’t keyboard-focusable and won’t have a clear name for screen readers. Consider using a `v-btn` with an icon, or adding `tabindex="0"`, an `@keydown` handler for Enter/Space, and an explicit accessible label via `aria-label` or `:title` (e.g. `"High disk usage – open disk tools"`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+3
to
12
| <v-icon | ||
| v-if="disk_usage_high" | ||
| v-tooltip="`High disk usage! (${disk_usage_percent.toFixed(1)}%) please clean up the disk`" | ||
| class="px-1 blinking white-shadow" | ||
| color="red" | ||
| @click="$router.push('/tools/disk')" | ||
| > | ||
| <template | ||
| #activator="{ on, attrs }" | ||
| > | ||
| <v-card | ||
| elevation="0" | ||
| color="transparent" | ||
| v-bind="attrs" | ||
| v-on="on" | ||
| > | ||
| <v-icon | ||
| v-if="disk_usage_high" | ||
| class="px-1 blinking white-shadow" | ||
| color="red" | ||
| :title="`High disk usage high! (${disk_usage_percent.toFixed(1)}%) please clean up the disk`" | ||
| > | ||
| mdi-content-save-alert | ||
| </v-icon> | ||
| </v-card> | ||
| </template> | ||
|
|
||
| <v-card | ||
| elevation="1" | ||
| width="350" | ||
| > | ||
| <v-container> | ||
| <div> | ||
| <table style="width: 100%"> | ||
| <tr> | ||
| <td>Percentage (Full):</td> | ||
| <td>{{ disk_usage_percent.toFixed(1) }} %</td> | ||
| </tr> | ||
| <tr> | ||
| <td>Free:</td> | ||
| <td> {{ disk_free_friendly }}</td> | ||
| </tr> | ||
| <tr> | ||
| <td>Total:</td> | ||
| <td> {{ disk_size_friendly }}</td> | ||
| </tr> | ||
| </table> | ||
| </div> | ||
| </v-container> | ||
| </v-card> | ||
| </v-menu> | ||
| mdi-content-save-alert | ||
| </v-icon> | ||
| <v-icon |
There was a problem hiding this comment.
issue (bug_risk): Clickable icon should be keyboard-accessible and have an explicit accessible label.
As this icon is now the sole way to reach the disk tools page, it needs to be fully accessible. A plain <v-icon> with only a click handler likely isn’t keyboard-focusable and won’t have a clear name for screen readers. Consider using a v-btn with an icon, or adding tabindex="0", an @keydown handler for Enter/Space, and an explicit accessible label via aria-label or :title (e.g. "High disk usage – open disk tools").
Member
|
tested with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Simplify the disk usage warning in the health tray menu to link directly to the disk tools page instead of showing an inline details popup.
Enhancements: