[backport/1.4] core:frontend:views:MainView: Fix check internet #3896
Open
patrickelectric wants to merge 1 commit intobluerobotics:1.4-devfrom
Open
Conversation
* Make sure the connect to internet banner only appears when no internet connection is available instead of WiFi network
Reviewer's guide (collapsed on small PRs)Reviewer's GuideBackports a fix to ensure the 'connect to internet' banner is driven by actual internet connectivity state instead of Wi‑Fi association state in MainView, wiring it to the helper store’s internet connection status. Sequence diagram for MainView rendering internet bannersequenceDiagram
actor User
participant Browser
participant VueApp
participant MainView
participant HelperStore
participant InternetConnectionState
User->>Browser: Navigate_to_MainView
Browser->>VueApp: Load_MainView_component
VueApp->>MainView: Create_instance
MainView->>HelperStore: Read_has_internet
HelperStore->>InternetConnectionState: Get_current_state
InternetConnectionState-->>HelperStore: state
HelperStore-->>MainView: has_internet_state
MainView->>MainView: Compute_has_internet(has_internet_state != OFFLINE)
MainView-->>VueApp: has_internet_value
VueApp->>Browser: Render_v_alert_v_if_!has_internet
Browser-->>User: Show_or_hide_connect_to_internet_banner
Class diagram for MainView internet connectivity checkclassDiagram
class MainView {
+computed menu_items : menuItem[]
+computed has_internet : boolean
}
class HelperStore {
+has_internet : InternetConnectionState
}
class InternetConnectionState {
<<enumeration>>
ONLINE
OFFLINE
CAPTIVE_PORTAL
UNKNOWN
}
MainView --> HelperStore : reads_has_internet
HelperStore --> InternetConnectionState : uses_state
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The computed
has_internetvalue currently treats any state other thanOFFLINEas online; ifInternetConnectionStatehas intermediate/unknown states (e.g.UNKNOWNorCHECKING), consider explicitly checking forONLINEto avoid hiding the banner during ambiguous connectivity. - Consider renaming the computed property from
has_internetto something likeisOnlineorinternetStatusOkto more clearly distinguish it from the underlyinghelper.has_internetstate and reduce confusion between the raw store value and the derived boolean.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The computed `has_internet` value currently treats any state other than `OFFLINE` as online; if `InternetConnectionState` has intermediate/unknown states (e.g. `UNKNOWN` or `CHECKING`), consider explicitly checking for `ONLINE` to avoid hiding the banner during ambiguous connectivity.
- Consider renaming the computed property from `has_internet` to something like `isOnline` or `internetStatusOk` to more clearly distinguish it from the underlying `helper.has_internet` state and reduce confusion between the raw store value and the derived boolean.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
I would hold this PR until we do the stable release for RadCam. |
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
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.
This is a backport from #3619 into 1.4
Summary by Sourcery
Bug Fixes: