-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add current AFD links #1652
Add current AFD links #1652
Conversation
@partly-igor The link on the forecast page is probably not styled properly at all, but I wasn't sure what to do with it. Can we chat about it tomorrow? For now it just shows above the daily forecast blocks, but I think it's supposed to have some kind of relationship with the weather story. |
Yeah definitely, we can figure out how much styling to apply and if we want a temporary header here |
@greg-does-weather I mocked up how the link can look on the current tab layout Desktop Mobile Let me know if anything is unclear, also happy to add the styling or pair. |
@partly-igor I think I got it. I was struggling with how to get the weather story and AFD link not to be full-width at the mobile breakpoint, but then I realized I could ditch the grid layout for mobile and just toss some margin in there. It worked a treat! |
e2b78ff
to
a02ebbd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small changes on the link itself, but otherwise looks great!
<use | ||
xlink:href="/{{ directory }}/assets/images/uswds/sprite.svg#info_outline" | ||
></use> | ||
</svg><a href="/afd/{{ weather.grid.wfo }}">Current Area Forecast Discussion</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two small things on the link:
- We should add a "usa-link" class to the link so that it gets the right colors
- For this instance of the link text I used just "Area Forecast Discussion" without the word "Current". Noting because then it doesn't wrap on the smallest screens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know what's annoying about that second bullet? I did that, and I lost the change when I accidentally messed up something else and I guess I ctrl-z'ed too much.
What does this PR do? 🛠️
Adds a link to the current AFD on WFO info pages and point forecast pages.