Allow user to specify and change location #110
Conversation
This file contains 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
Now that we have the zones seeded in the DB, we don’t need the JSON file.
We could and probably should do this with Stimulus, but I think that’s not necessary in what is already a large ticket, so I am leaving it for a subsequent refactor.
…r methods Because we want to be able to navigate directly to a forecast for a given day and zone, we need our show and update methods to be set up to expect these parameters, and to set them as instance variables. We also need to specify default cases where one or both parameters are not specified.
Now that we have a method to return the selected day or else return “today”, this check is unnecessary.
Before we move our tab change handling from Stimulus to Turbo, we need to be able to calculate the CSS classes for each tab within our view component. When the tabs are re-rendering on every update, we can apply the classes from the server in Rails, in code that we have tested, rather than doing it on the fly in Javascript and relying on more expensive integration test coverage.
Now that we are re rendering the tabs as well within our Turbo Stream, we can remove the alert_guidance turbo frame, because this sits within the pollution_forecast partial. I have also created a separate partial, `selected_day_input`, to capture the currently selected day within the form that updates the forecast to a new location. This would be messy without its own turbo frame and template, because it would require significant inline ERB. I have also renamed forecast_tabs to pollution_forecast, to distinguish it from the UV, pollen and temperature forecast, and capture the fact that the component does not only consist of tabs (it also includes the alert guidance).
This has been superseded by our turbo stream, so is now redundant.
This extends Turbo Streams with a number of actions, including push_state, which will allow us to add query parameters for day and zone to the URL when we trigger our Turbo Stream.
Now we are in a position to test that specifying the zone and day as URL parameters is sufficient to display the forecast for the given zone and day.
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.
Changes in this PR
This PR allows users to change location using the zone dropdown menu, ensuring that the forecasts update to the given zone. The previously selected day will be maintained, i.e. if the user is looking at tomorrow's forecast, and then changes location, they will now be viewing the new location's forecast for tomorrow.
This has been achieved by moving the location menu and forecast tabs within the Turbo Stream update for the forecast. Previously the styling of the forecast tabs was being handled with Stimulus.
This PR also allows the entire forecast view to be driven by state in the URL. Now, if the URL specifies a zone and/or day as URL parameters, the forecast for that zone and day will be displayed. If no zone is specified, Southwark will be shown as the default. If not day is specified, the forecast will default to today.
Screenshots of UI changes
Before
After
Next steps
accessibility testing
on this feature.