Add styling to the WFO page #1539

greg-does-weather · 2024-08-07T21:55:34Z

What does this PR do? 🛠️

This extends the previous work standing up a WFO page by adding the styling described in Figma.

This PR makes one extension to the previous approach to rendering WFO pages. If an office has not create a WFO information content node, we still show that WFO's page using the information available to us – its name and coverage area map. Requests for WFO pages for offices that don't exist continues to result in a 404.

closes #1474

Screenshots (if appropriate): 📸

Full content, mobile size

Full content, desktop size

Default content, mobile size

Default content, desktop size

github-actions · 2024-08-07T21:55:52Z

This PR modifies theme Javascript or CSS assets but does not update the theme libraries file. Did you mean to update the appropriate version information in the libraries file?

github-actions · 2024-08-07T22:14:43Z

This PR modifies theme Javascript or CSS assets but does not update the theme libraries file. Did you mean to update the appropriate version information in the libraries file?

github-actions · 2024-08-08T15:45:41Z

This PR modifies theme Javascript or CSS assets but does not update the theme libraries file. Did you mean to update the appropriate version information in the libraries file?

github-actions · 2024-08-08T15:46:01Z

This PR modifies theme Javascript or CSS assets but does not update the theme libraries file. Did you mean to update the appropriate version information in the libraries file?

github-actions · 2024-08-08T15:57:12Z

This PR modifies theme Javascript or CSS assets but does not update the theme libraries file. Did you mean to update the appropriate version information in the libraries file?

github-actions · 2024-08-08T18:12:20Z

This PR modifies theme Javascript or CSS assets but does not update the theme libraries file. Did you mean to update the appropriate version information in the libraries file?

eric-gade

This looks good and works well on my end 👍

Just a couple of things:

Renaming the individual field templates means those fields will display the same way where ever they are (re)used. I think this is fine, but something to consider when building out the WFO Promo block, for example, where we can probably use some kind of drupal shortcuts to just render the fields according to existing templates if we want.

I'll leave most of the styling stuff to @partly-igor , but I did notice that there probably needs to be some margin on the bottom in the mobile/tablet view near the forecast discussion (though I know that is work that is forthcoming):

Code Review Checklist

This is an automated comment on every pull request requiring a review. A checked item is either an assertion that I have tested this item or an indication that I have verified it does not apply to this pull request.

The Basics

Checks are passing
I read the code
I ran the code
(if applicable) Post deploy steps are run
(if applicable) I validated the change on the deployed version in

Documentation

changes to “how we do things” are documented in READMEs
all new functions and methods are commented using plain language
any new modules added documented in modules.md

Security

security false positives are documented
data from external sources is cleaned and clearly marked

Reliability

error handling exists for unusual or missing values
interactions with external systems are wrapped in try/except
functionality is tested with unit or integration tests
dependency updates in composer.json also got changed in composer-lock.json

Infrastructure

all changes are auditable and documented via a script
it is clear who can and should run the script
(if applicable) diagrams have been updated or added in PlantUML

Accessibility

Device Matrix

firefox/gecko (renders correctly and user interactions work)
chrome/chromium/edge (renders correctly and user interactions work)
safari/webkit (renders correctly and user interactions work)
web page is readable and usable
- at 480px (mobile)
- at 640px (tablet)
- at 1024px (desktop)

eric-gade · 2024-08-12T12:55:15Z

web/modules/weather_routes/src/Controller/WeatherInfoController.php

+            if (!$wfoInfo) {
+                // If there's not already a node for this WFO, create a blank
+                // one in memory so we can still render a page. It'll just have
+                // less content in it.
+                $wfoInfo = Node::create(["type" => "wfo_info"]);
+                $wfoInfo->set("field_wfo", $wfoTerm);
+            }


eric-gade · 2024-08-12T12:59:53Z

web/themes/new_weather_theme/templates/wfo-info/node--wfo-info.html.twig

+    {% set hasPhone = phone is not empty %}
+    {% set hasAddress = address is not empty %}
+    {% set hasEmail = email is not empty %}
+
+    {% set hasFacebook = not (content.field_facebook_url.isEmpty or content.field_facebook_url.0 is not defined) %}
+    {% set hasTwitter = not (content.field_twitter_url.isEmpty or content.field_twitter_url.0 is not defined) %}
+    {% set hasYoutube = not (content.field_youtube_url.isEmpty or content.field_youtube_url.0 is not defined) %}
+    {% set hasSocial = hasFacebook or hasTwitter or hasYoutube %}
+
+    {% set hasContact = hasSocial or hasAddress or hasPhone or hasEmail %}


I think we should make a small ticket for the future that moves this logic into the hook where we are already massaging the data for display in twig. What do you think?

partly-igor

Looking good, there are some design/style updates to make it match the design.

partly-igor · 2024-08-12T18:17:35Z

spatial-data/maps/template.html

+        // one step so we have a somewhat broader view.
+        const bounds = wfo.getBounds();
+        map.fitBounds(bounds);
+        map.zoomOut(1);


I am curious to see what it looks like without this one step zoom out – not sure what the right level is, but I am wondering if the current one is a touch too zoomed out.

web/themes/new_weather_theme/templates/wfo-info/node--wfo-info.html.twig

partly-igor · 2024-08-12T18:44:02Z

web/themes/new_weather_theme/templates/wfo-info/node--wfo-info.html.twig

+      stuff, make sure we add things to the translations list appropriately.
+    #}
+    <div class="grid-row">
+      <div class="grid-col-12 tablet:grid-col-6">


On mobile the spacing between the sections is a uneven. Could we make it so there is 4 units of space between the between the bottom of one section and the start (border) of the next

👍 I added margin-top-4 padding-top-4 to all the headings and that seemed to accomplish it?

Yep, that looks great, only one that sticks out is the list under Local Expertise, so I think we add margin-bottom-0 to that ul we should be good to go

web/themes/new_weather_theme/templates/wfo-info/node--wfo-info.html.twig

greg-does-weather · 2024-08-12T19:45:27Z

With automatic zoom out:

Without automatic zoom out:

I think I agree, that extra zoom out is unnecessary. I'm curious if it'll be dependent on the particular WFO, but I don't think so – this approach should guarantee that the entire WFO is visible before we do any zooming, so it should be okay?

partly-igor · 2024-08-12T19:48:54Z

Nice yeah, I think that without the zoom out is a good balance.

partly-igor

Updates look really good, except the padding below the border should be 105 instead of 4

partly-igor · 2024-08-12T19:54:51Z

web/themes/new_weather_theme/templates/wfo-info/node--wfo-info.html.twig

@@ -33,31 +33,31 @@
    #}
    <div class="grid-row">
      <div class="grid-col-12 tablet:grid-col-6">
-        <h2 class="border-top-1px border-base-light text-normal text-primary-darker padding-top-1">
+        <h2 class="font-sans-xl border-top-1px border-base-light text-normal text-primary-darker padding-top-4 margin-top-4">


padding-top-105

partly-igor · 2024-08-12T19:54:54Z

web/themes/new_weather_theme/templates/wfo-info/node--wfo-info.html.twig

        </ul>
      </div>

      <div class="grid-col-12 tablet:grid-col-6">
-        <h2 class="border-top-1px border-base-light text-normal text-primary-darker padding-top-1">
+        <h2 class="font-sans-xl border-top-1px border-base-light text-normal text-primary-darker padding-top-4 margin-top-4">


padding-top-105

partly-igor · 2024-08-12T19:55:03Z

web/themes/new_weather_theme/templates/wfo-info/node--wfo-info.html.twig

@@ -73,7 +73,7 @@
    {% set hasContact = hasSocial or hasAddress or hasPhone or hasEmail %}

    {% if hasContact %}
-      <h2 class="border-top-1px border-base-light text-normal text-primary-darker padding-top-1 margin-bottom-0">
+      <h2 class="font-sans-xl border-top-1px border-base-light text-normal text-primary-darker margin-bottom-0 padding-top-4 margin-top-4">


padding-top-105

partly-igor

Updates look good!

Base automatically changed from eg-1470-wfo-info-routing to main August 8, 2024 15:40

greg-does-weather force-pushed the mgwalker/1474-wfo-page-styling branch from ec798b2 to 614b42d Compare August 8, 2024 15:45

greg-does-weather force-pushed the mgwalker/1474-wfo-page-styling branch from ebdcff3 to 9fecb8d Compare August 8, 2024 18:11

greg-does-weather changed the title ~~Mgwalker/1474 wfo page styling~~ Add styling to the WFO page Aug 8, 2024

greg-does-weather requested review from eric-gade and partly-igor August 8, 2024 18:18

greg-does-weather marked this pull request as ready for review August 8, 2024 18:19

eric-gade approved these changes Aug 12, 2024

View reviewed changes

greg-does-weather added 9 commits August 12, 2024 12:47

styling part 1

08a7154

image scaling

f013f51

generate wfo coverage maps

4d9814b

styling!

fdfe4be

add email address

68f3fba

translation-ready-ize

837b0bb

comments

3baef14

bump style version

ba4f593

apply workflow to wfo info content type

4a82b93

greg-does-weather force-pushed the mgwalker/1474-wfo-page-styling branch from 9d443a1 to 4a82b93 Compare August 12, 2024 17:51

partly-igor requested changes Aug 12, 2024

View reviewed changes

style updates/fixes

41f67b7

lint fix

789f781

partly-igor reviewed Aug 12, 2024

View reviewed changes

greg-does-weather added 2 commits August 12, 2024 15:08

update maps

217f9ff

style fixes

92b2404

partly-igor self-requested a review August 13, 2024 14:35

partly-igor approved these changes Aug 13, 2024

View reviewed changes

greg-does-weather enabled auto-merge August 13, 2024 15:34

Merge branch 'main' into mgwalker/1474-wfo-page-styling

71ce978

greg-does-weather merged commit cc13b3e into main Aug 13, 2024
18 checks passed

greg-does-weather deleted the mgwalker/1474-wfo-page-styling branch August 13, 2024 15:42

This was referenced Aug 13, 2024

Modify WFO promo block to serve data from the WFO info content type #1555

Merged

[WFO-CMS-Data] Add WFO map to WFO page template #1472

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add styling to the WFO page #1539

Add styling to the WFO page #1539

greg-does-weather commented Aug 7, 2024 •

edited

Loading

github-actions bot commented Aug 7, 2024

github-actions bot commented Aug 7, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

eric-gade left a comment

eric-gade Aug 12, 2024

eric-gade Aug 12, 2024

partly-igor left a comment

partly-igor Aug 12, 2024

partly-igor Aug 12, 2024

greg-does-weather Aug 12, 2024

partly-igor Aug 12, 2024

greg-does-weather commented Aug 12, 2024 •

edited

Loading

partly-igor commented Aug 12, 2024

partly-igor left a comment

partly-igor Aug 12, 2024

partly-igor Aug 12, 2024

partly-igor Aug 12, 2024

partly-igor left a comment

Add styling to the WFO page #1539

Add styling to the WFO page #1539

Conversation

greg-does-weather commented Aug 7, 2024 • edited Loading

What does this PR do? 🛠️

Screenshots (if appropriate): 📸

github-actions bot commented Aug 7, 2024

github-actions bot commented Aug 7, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

eric-gade left a comment

Choose a reason for hiding this comment

Code Review Checklist

The Basics

Documentation

Security

Reliability

Infrastructure

Accessibility

Device Matrix

Choose a reason for hiding this comment

Choose a reason for hiding this comment

partly-igor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greg-does-weather commented Aug 12, 2024 • edited Loading

With automatic zoom out:

Without automatic zoom out:

partly-igor commented Aug 12, 2024

partly-igor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

partly-igor left a comment

Choose a reason for hiding this comment

greg-does-weather commented Aug 7, 2024 •

edited

Loading

greg-does-weather commented Aug 12, 2024 •

edited

Loading