Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AFD Styling #1621

Merged
merged 11 commits into from
Aug 26, 2024
Merged

AFD Styling #1621

merged 11 commits into from
Aug 26, 2024

Conversation

eric-gade
Copy link
Collaborator

@eric-gade eric-gade commented Aug 23, 2024

What does this PR do? 🛠️

This PR implements the styling specified in #1527 and probably #1530

### READ BEFORE MERGING! ###
This branch is itself branched from eg-1528-afd-html-endpoint, which as of this writing is currently subject to an open PR #1587. That PR needs to be merged into main before we can merge this one in. I would prefer that, once the other PR is merged, we rebase this branch and force push (otherwise it might junk up the history). But either way, it is a prerequisite.

Done!

What does the reviewer need to know? 🤔

Sticking with basic HTTP server requests / adding submit button

It was with this PR and related tickets that we have decided, for now, to stick with basic server-side/form based HTTP requests for updating the WFO selector and AFD version selector. Because of this, we called an audible and added a submit button that is not present in the original design Figma.

Parsed TEMPS/POPS table

Because it seemed like low hanging fruit, we have made an initial pass at parsing the (sometimes present) TEMPS/POPS table from raw text into an actual semantic html table. This can be iterated upon in the future.

Lack of cycling header

We have not yet included the "cycling header" from the Figma design. This is for a couple of reasons:

  • We need to make changes on the backend in order to properly provide URLs for the "previous" and "next" versions so that the arrow buttons display and work logically;
  • The design calls for specifying the State that a given WFO is located in. This is a two-front problem. First, the state data is not availble in the Drupal taxonomy. Second -- and related to the first -- WFOs can sometimes cover multiple States. So that part of the design needs a slight iteration.

We have captured the need for implementing this component in this issue.

Screenshots (if appropriate): 📸

Desktop:

localhost_8080_afd_TAE_12c9be7e-414e-4fae-a0a1-b02e50ff2070

Mobile:

localhost_8080_afd_TAE_12c9be7e-414e-4fae-a0a1-b02e50ff2070(iPhone XR)

Copy link
Contributor

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?

@greg-does-weather greg-does-weather marked this pull request as ready for review August 26, 2024 19:18
Copy link
Contributor

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?

Copy link
Collaborator

@greg-does-weather greg-does-weather left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Showtime!

Copy link
Contributor

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?

@greg-does-weather greg-does-weather merged commit be334e0 into main Aug 26, 2024
17 of 18 checks passed
@greg-does-weather greg-does-weather deleted the eg-1527-afd-initial-styling branch August 26, 2024 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants