Implement air quality alerts in styled forecasts view #76

patrickjfl · 2024-11-01T11:55:38Z

Changes in this PR

This PR implements air quality alerts within the air pollution tabs, as well as rendering guidance when there is an alert in place. The styles are updated for these components, as is the implementation of turbo stream, allowing us to update multiple HTML elements with a single request.

Screenshots of UI changes

Before

After

Next steps

Run any necessary
accessibility testing
on this feature.

edavey

Really nice work, very clearly structured.

However, I do think it's wrong for the #daqi_indicator_colour_class to reside in AirPollutionPrediction -- that it in fact belongs in a view component. Perhaps the right view component would be DaqiMarkerComponent?

tailwind.config.js

app/models/air_pollution_prediction.rb

edavey · 2024-11-04T12:17:36Z

spec/factories/air_pollutions_predictions.rb

@@ -53,6 +53,96 @@
      ozone { 10 }
    end

+    trait 1 do


nice use of FactoryBot traits!

spec/models/air_pollution_prediction_spec.rb

app/javascript/controllers/tab_controller.js

app/views/styled_forecasts/_day_tab.html.erb

edavey

Looking good!

Now that the styling of the day tabs is becoming more complex it seems like a good opportunity to move that logic into the component -- and see if perhaps there are actually several class-related methods needed, rather than one very complicated one! This in turn may lead to simpler CSS rules?

tailwind.config.js

app/components/day_tab_component.html.erb

As these class names will be dynamically generated in the template, we need to explicitly include all of them in our Tailwind config file ‘safelist’, or else they will not be included in the build: https://tailwindcss.com/docs/content-configuration#safelisting-classes

This approach, of returning via a turbo stream template, gives us more flexibility to apply changes to multple distinct elements within the page. In our case, we want to re render the pollen, UV and temperature predictions, but we’ll also want to re render the alert guidance component in the next commit.

This will be triggered by the turbo stream in the previous commit, and will be rendered when there is an air quality alert. I have also removed the bold tags from the guidance text as they are not part of the designs.

The rules that govern which styles should apply are quite complex, so I have ended up with a slightly verbose implementation that hopefully is at least fairly clear. The order in which the classes are listed is also important - the ‘inactive’ class needs to be below ‘active’ in order to override it. There are two versions of each of the DAQI alert classes. This is unfortunate but allows us to simplify the Javascript code that toggles the selected classes.

We will use this in our air pollution tabs when there is an alert.

This component will allow us to store the logic required to calculate the styling required, which vary according to each DAQI level. Add background colour class util to air pollution prediction We will use this to generate the colour used in the forecast tabs.

We now need to toggle an inactive class as well as an active class, because of the interaction of the active class with the daqi-level classes. In cases of alerts, we want to override the active class with the relevant daqi-level class, but where that tab is not active, we want to override that again with our inactive styling.

The integration tests in CI were complaining about overlapping elements on a button because of he icon on the new air pollution tab. It doesn’t matter that it overlaps, so I’ve introduced this alternative syntax to fix the issue.

patrickjfl · 2024-11-05T16:33:32Z

Implement air quality alerts in styled forecasts view

edavey

Really interesting to see how you've extended the Stimulus controller to handle this complexity. As you note in your commit message, you may find it easier to re-render from the server when you next come to add more complexity to this area!

edavey · 2024-11-05T16:39:11Z

app/javascript/controllers/tab_controller.js

+    if (
+      this.dayTarget.innerText !== "Today" &&
+      this.daqiValueTarget.innerText[6] > 3
+    ) {
+      this.addDaqiAlertClass();
+    }


this might perhaps be clearer if you extracted to a function with an intention-revealing name e.g. addAlertClassIfTodayHasAirQualityAlert()?

yes, although it's more like addAlertClassIfNotTodayAndHasAirQualityAlert 😄

maybe that is ok though, even if it is long

edavey · 2024-11-05T16:41:15Z

app/javascript/controllers/tab_controller.js

+  removeDaqiAlertClasses() {
+    document.querySelectorAll(".tabs .tab").forEach((el) => {
+      const daqiAlertClassRegex = new RegExp(
+        "daqi-alert-after-today-selected-level-(\\d{1,2})"
+      );
+
+      const fullClassName = Array.from(el.classList).find((className) =>
+        className.match(daqiAlertClassRegex)
+      );
+
+      if (fullClassName) {
+        el.classList.remove(fullClassName);
+      }
+    });


clever! bravo 👏

If today has an alert, we always want to display the alert stylings. For tomorrow and the day after tomorrow, if they have alerts, we only want to display the alert stylings when the tab is selected. I have opted for a slightly verbose Javascript implementation that extends the Stimulus code that was already in place, rather than bringing the tabs into the Turbo stream that is already in place for this action. However, if any part of this design becomes any more complex, this would probably be a good candidate to refactor along those lines.

We need to update our integration tests to allow us to explicitly set DAQI values, so that we can check that the correct class for that value has been set on the active tab where there is an alert.

patrickjfl requested review from edavey and jdudley1123 November 1, 2024 11:55

patrickjfl force-pushed the 94-implement-air-quality-alerts-in-styled-forecasts-view branch 2 times, most recently from 9c6573f to b33bab8 Compare November 1, 2024 15:33

patrickjfl marked this pull request as ready for review November 1, 2024 15:39

edavey reviewed Nov 4, 2024

View reviewed changes

patrickjfl force-pushed the 94-implement-air-quality-alerts-in-styled-forecasts-view branch from b33bab8 to bfabff4 Compare November 5, 2024 10:05

edavey approved these changes Nov 5, 2024

View reviewed changes

tailwind.config.js Show resolved Hide resolved

app/components/day_tab_component.html.erb Outdated Show resolved Hide resolved

patrickjfl added 10 commits November 5, 2024 16:07

Create partial for rendering alert guidance

f351951

This will be triggered by the turbo stream in the previous commit, and will be rendered when there is an air quality alert. I have also removed the bold tags from the guidance text as they are not part of the designs.

Add exclamation triangle icon

d684306

We will use this in our air pollution tabs when there is an alert.

Replace day_tab partial with new view component

9820a51

Add integration tests for air quality alerts

2d0c48a

Fix Cuprite error

16567ec

The integration tests in CI were complaining about overlapping elements on a button because of he icon on the new air pollution tab. It doesn’t matter that it overlaps, so I’ve introduced this alternative syntax to fix the issue.

patrickjfl force-pushed the 94-implement-air-quality-alerts-in-styled-forecasts-view branch from bfabff4 to a8cdf0b Compare November 5, 2024 16:08

edavey approved these changes Nov 5, 2024

View reviewed changes

patrickjfl force-pushed the 94-implement-air-quality-alerts-in-styled-forecasts-view branch from a8cdf0b to 66f25fa Compare November 5, 2024 16:52

patrickjfl added 2 commits November 6, 2024 09:28

Update integration tests to check for alert classes

6eca99a

We need to update our integration tests to allow us to explicitly set DAQI values, so that we can check that the correct class for that value has been set on the active tab where there is an alert.

patrickjfl force-pushed the 94-implement-air-quality-alerts-in-styled-forecasts-view branch from 66f25fa to 6eca99a Compare November 6, 2024 09:30

patrickjfl merged commit 58b3755 into develop Nov 6, 2024
2 checks passed

patrickjfl deleted the 94-implement-air-quality-alerts-in-styled-forecasts-view branch November 6, 2024 09:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement air quality alerts in styled forecasts view #76

Implement air quality alerts in styled forecasts view #76

patrickjfl commented Nov 1, 2024

edavey left a comment

edavey Nov 4, 2024

edavey left a comment •

edited

Loading

patrickjfl commented Nov 5, 2024

edavey left a comment

edavey Nov 5, 2024

patrickjfl Nov 5, 2024

patrickjfl Nov 5, 2024

edavey Nov 5, 2024

Implement air quality alerts in styled forecasts view #76

Implement air quality alerts in styled forecasts view #76

Conversation

patrickjfl commented Nov 1, 2024

Changes in this PR

Screenshots of UI changes

Before

After

Next steps

edavey left a comment

Choose a reason for hiding this comment

edavey Nov 4, 2024

Choose a reason for hiding this comment

edavey left a comment • edited Loading

Choose a reason for hiding this comment

patrickjfl commented Nov 5, 2024

edavey left a comment

Choose a reason for hiding this comment

edavey Nov 5, 2024

Choose a reason for hiding this comment

patrickjfl Nov 5, 2024

Choose a reason for hiding this comment

patrickjfl Nov 5, 2024

Choose a reason for hiding this comment

edavey Nov 5, 2024

Choose a reason for hiding this comment

edavey left a comment •

edited

Loading