frontend: wifi: add refresh wifi list button#3438
frontend: wifi: add refresh wifi list button#3438nicoschmdt wants to merge 1 commit intobluerobotics:masterfrom
Conversation
Reviewer's GuideAdds a refresh button to the WifiManager toolbar that emits a refresh-request event, and wires it up in the WifiTrayMenu to invoke fetchAvailableNetworks on the wifi-updater component. Sequence diagram for the WiFi list refresh interactionsequenceDiagram
actor User
participant WifiManager
participant WifiTrayMenu
participant WifiUpdater
User->>WifiManager: Clicks refresh button
WifiManager->>WifiTrayMenu: emit refresh-request
WifiTrayMenu->>WifiUpdater: fetchAvailableNetworks()
WifiUpdater-->>WifiTrayMenu: (updates WiFi list)
WifiTrayMenu-->>User: Updated WiFi list displayed
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.
Hey @nicoschmdt - I've reviewed your changes - here's some feedback:
- Interacting with the WifiUpdater via $refs can be brittle—consider having WifiUpdater listen for a shared event or use a Vuex action instead of directly calling its method.
- The hide-details prop is not applicable to v-btn and can be removed to clean up the button configuration.
- Avoid hardcoding color="gray" on the refresh button—use a theme variable (e.g. primary or default) to keep styling consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Interacting with the WifiUpdater via $refs can be brittle—consider having WifiUpdater listen for a shared event or use a Vuex action instead of directly calling its method.
- The hide-details prop is not applicable to v-btn and can be removed to clean up the button configuration.
- Avoid hardcoding color="gray" on the refresh button—use a theme variable (e.g. primary or default) to keep styling consistent.
## Individual Comments
### Comment 1
<location> `core/frontend/src/components/wifi/WifiManager.vue:13` </location>
<code_context>
>
<v-toolbar-title>Wifi</v-toolbar-title>
<v-spacer />
+ <v-btn
+ v-tooltip="'Refresh'"
+ icon
+ color="gray"
+ hide-details="auto"
+ @click="$emit('refresh-request')"
+ >
+ <v-icon>mdi-refresh</v-icon>
+ </v-btn>
<v-btn
</code_context>
<issue_to_address>
Button color 'gray' may not match Vuetify theme.
Consider using a standard Vuetify theme color or ensure 'gray' is defined in your theme to maintain consistent styling.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
<v-btn
v-tooltip="'Refresh'"
icon
color="gray"
hide-details="auto"
@click="$emit('refresh-request')"
>
<v-icon>mdi-refresh</v-icon>
</v-btn>
=======
<v-btn
v-tooltip="'Refresh'"
icon
color="primary"
hide-details="auto"
@click="$emit('refresh-request')"
>
<v-icon>mdi-refresh</v-icon>
</v-btn>
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
8dfd5f5 to
7f1b764
Compare
|
I have attached a video of the current refresh behaviour on the description. I'm not too sure if I like having the message "No wifi networks available" when rescanning but it is a minor nitpick |
|
@ES-Alexander and @Williangalvani what do you guys think? |
ES-Alexander
left a comment
There was a problem hiding this comment.
Generally looks good to me - nice work!
A small suggestion on the tooltip wording, just to make it clearer that the refresh button is not a restart button for the current connection :-)
fix #1863
Refresh:
https://github.com/user-attachments/assets/93d98ccc-7d2c-4e47-8589-a8b09eafcc67
Summary by Sourcery
Add a manual refresh button to the WifiManager component and wire it up in the WifiTrayMenu to reload available networks on demand
New Features: