-
Notifications
You must be signed in to change notification settings - Fork 119
Add missing tooltips #3716
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
Add missing tooltips #3716
Conversation
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Reviewer's GuideAdds missing Vuetify tooltips to several tray menu icons and introduces computed tooltip text for pirate mode and Ethernet connectivity. Class diagram for updated tray menu tooltip logicclassDiagram
class PirateModeTrayMenu {
+boolean show_menu
+Settings settings
+pirate_mode_tooltip() string
+showMenu(show boolean) void
}
class EthernetTrayMenu {
+available_interfaces list~EthernetInterface~
+interface_icon() string
+interface_connected_tooltip() string
}
class BeaconTrayMenu {
+tooltip_text string
}
class OnBoardComputerRequiredTrayMenu {
+icon mdi_restart_alert
}
class SystemCheckerTrayMenu {
+icon mdi_alert
}
class VehicleRebootRequiredTrayMenu {
+icon mdi_restart_alert
}
class CloudTrayMenu {
+icon cloud_sync_status
}
class Settings {
+boolean is_pirate_mode
}
class EthernetInterfaceInfo {
+boolean connected
}
class EthernetInterface {
+EthernetInterfaceInfo info
}
PirateModeTrayMenu --> Settings : uses
EthernetTrayMenu "*" --> EthernetInterface : filters
EthernetInterface --> EthernetInterfaceInfo : has
PirateModeTrayMenu ..> v-tooltip_pirate_mode_tooltip : tooltip_binding
EthernetTrayMenu ..> v-tooltip_interface_connected_tooltip : tooltip_binding
BeaconTrayMenu ..> v-tooltip_tooltip_text : tooltip_binding
OnBoardComputerRequiredTrayMenu ..> v-tooltip_static_text : tooltip_binding
SystemCheckerTrayMenu ..> v-tooltip_static_text : tooltip_binding
VehicleRebootRequiredTrayMenu ..> v-tooltip_static_text : tooltip_binding
CloudTrayMenu ..> v-tooltip_static_text : tooltip_binding
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey - I've left some high level feedback:
- In
PirateModeTrayMenu.vue, consider referencingthis.settingsinside thepirate_mode_tooltipcomputed property for consistency with the rest of the component and to make reactivity more obvious. - In
EthernetTrayMenu.vue, the logic to computeconnected_interfacesis duplicated betweeninterface_connected_iconandinterface_connected_tooltip; extracting this into a shared computed/getter would reduce repetition and potential divergence.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `PirateModeTrayMenu.vue`, consider referencing `this.settings` inside the `pirate_mode_tooltip` computed property for consistency with the rest of the component and to make reactivity more obvious.
- In `EthernetTrayMenu.vue`, the logic to compute `connected_interfaces` is duplicated between `interface_connected_icon` and `interface_connected_tooltip`; extracting this into a shared computed/getter would reduce repetition and potential divergence.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| v-on="on" | ||
| > | ||
| <v-icon | ||
| v-tooltip="tooltip_text" |
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.
I believe this should be a bind just like it was?
| v-tooltip="tooltip_text" | |
| :v-tooltip="tooltip_text" |
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.
That's not necessary
| v-on="on" | ||
| > | ||
| <v-icon | ||
| v-tooltip="interface_connected_tooltip" |
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.
shouldn't it be a bind?
| v-tooltip="interface_connected_tooltip" | |
| :v-tooltip="interface_connected_tooltip" |
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.
That's not necessary
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.
I think I'll never be used to Vue 😭
Fix #3613
Summary by Sourcery
Add descriptive tooltips to various tray menu icons to clarify their status and actions.
New Features:
Bug Fixes: