Add offline hourly weather fallback using stored forecast data #116#123
Open
numericals-org wants to merge 2 commits into
Open
Add offline hourly weather fallback using stored forecast data #116#123numericals-org wants to merge 2 commits into
numericals-org wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughSubstantial rewrite of weather module logic in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WeatherUI as Weather Module
participant API as Weather/Forecast API
participant Storage as localStorage
participant Network
User->>WeatherUI: Enter location/open app
WeatherUI->>WeatherUI: Check cache validity
alt Cache Valid & Online
WeatherUI->>Storage: Retrieve cached weather data
WeatherUI->>WeatherUI: Render from cache
WeatherUI->>User: Display weather
else Offline
WeatherUI->>Storage: Retrieve cached forecast data
WeatherUI->>WeatherUI: Extract current hour from forecast
WeatherUI->>User: Render offline weather from cache
else Online & Needs Refresh
User->>WeatherUI: Request location suggestions
WeatherUI->>API: Fetch location suggestions
API-->>WeatherUI: Return suggestions list
WeatherUI->>User: Display suggestions
User->>WeatherUI: Select location
WeatherUI->>API: Fetch weather & forecast data
API-->>WeatherUI: Return weather & hourly forecast
WeatherUI->>Storage: Cache weather data & forecast
WeatherUI->>User: Render updated weather
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@scripts/weather.js`:
- Around line 466-483: getForecastForNow() can return undefined causing a crash
when accessing properties; in the offline branch that checks !navigator.onLine,
first guard that the result of getForecastForNow() (from
parsedData.forecast.forecastHours) exists and early-return or fall back to
parsedData.current, then read the hourly icon from the hourly object (use
data.condition.icon) instead of parsedData.current.condition.icon; update the
same guard/fallback logic for the similar offline block around the later code
(the block at ~607-612) to avoid undefined access.
- Around line 74-80: The code loads savedLocation from weatherParsedLocation but
saveLocButton writes weatherLocation, causing manual entries to be ignored;
change the load and assignment so user input and currentUserLocation use
weatherLocation (e.g., read localStorage.getItem("weatherLocation") into
savedLocation and assign userLocInput.value and currentUserLocation from that),
and reserve weatherParsedLocation only for parsed/cache validation logic; update
any save paths (saveLocButton handler, and places that set currentUserLocation)
to consistently read/write weatherLocation while leaving weatherParsedLocation
for parsed-cache checks.
- Around line 90-103: The Enter key handler handleEnterPress currently triggers
a button click even when the suggestion list has focus; update handleEnterPress
(used by userAPIInput and userLocInput) to first detect if a suggestion is
active and, if so, return without clicking. Implement the gate by checking
common suggestion indicators used by your UI (for example: an element with a
highlighted/active suggestion class, an open suggestion list role/aria-expanded
on the input, or an aria-activedescendant on the input) and only call
document.getElementById(buttonId).click() when no active suggestion is present.
- Around line 391-399: The refresh condition currently triggers a network fetch
even when offline (so the thrown fetch error prevents falling back to cached
UI); update the refresh gating around the block that checks parsedData,
weatherParsedTime, weatherParsedLocation, and weatherParsedLang to require
navigator.onLine before attempting a network refresh, and if navigator.onLine is
false allow rendering cached parsedData (even if stale) instead of attempting
fetch; ensure retentionTime logic (retentionTime variable) remains used only to
decide freshness when online and that fetch attempts are only made when
navigator.onLine is true.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/weather.js
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/weather.js (2)
scripts/languages.js (3)
translations(10-42)isRTL(364-364)feelsLikeElement(376-376)scripts/alert-modal.js (2)
agreeText(151-151)cancelText(152-152)
🔇 Additional comments (1)
scripts/weather.js (1)
943-948: The scope analysis in this comment is incorrect.minMaxTempCheckboxis declared at the top-level (line 82), not insidegetWeatherData. Since both the declaration and event handler are in the same scope, there is no ReferenceError risk here. No changes are needed to this code.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Author
|
Morning @prem-k-r Just following up on this PR in case it was missed. Whenever you have time, I’d appreciate your feedback. Thanks! |
Owner
|
Sure, I'll have a look next week |
Author
|
Thank you, I appreciate it! |
|
@prem-k-r can you please assign this pr to me ? Thank you. |
Owner
Not at the moment. I haven’t reviewed this PR yet, and it’s not in an abandoned state that can be discarded where it can be reassigned. |
Author
|
Morning @prem-k-r |
prem-k-r
added
enhancement
Author
|
Morning @prem-k-r |
Owner
|
@numericals-org, apologies I am busy in work, I'll take a look when I got some free time for the review. |
|
Morning @prem-k-r |
Author
|
Morning @prem-k-r |
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
4 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.
Switch Weather Data Handling to Use Forecast Endpoint Fully (Offline Support) #116
📌 Description
This PR improves the weather feature by utilizing the hourly forecast data already returned by the WeatherAPI
forecastendpoint.In addition to the current weather, the extension now stores today’s hourly forecast locally and uses it as a fallback when the user is offline.
When offline, the extension displays the forecast for the current hour (e.g., 12:15 → shows 12:00 data) instead of showing no weather information.
This enhances usability, reliability, and the overall user experience without increasing API usage.
🎨 Visual Changes (Screenshots / Videos)
#Offline
#Online
🔗 Related Issues
✅ Checklist
Summary
This PR adds offline support to the weather feature by implementing an hourly forecast fallback mechanism. When the user is offline, the extension displays weather information for the current hour using cached forecast data from the WeatherAPI endpoint, rather than showing no weather information.
Changes
Core Feature:
weatherParsedLocation,weatherParsedData,weatherParsedTime,weatherParsedLang)Weather Data Management:
getWeatherDataflow with more modular rendering and caching logicLocation and API Integration:
UI and Localization:
defaultWeather.svgon errorTechnical Details
scripts/weather.js(+871/-544 lines)Impact