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

Update USA Banner component #37

Merged
merged 8 commits into from
Sep 3, 2024
Merged

Update USA Banner component #37

merged 8 commits into from
Sep 3, 2024

Conversation

katyasoup
Copy link
Collaborator

@katyasoup katyasoup commented Aug 29, 2024

resolves #33

What this PR changes:

  • adds a new route to enable htmx swapping for show/hide functionality on the USA banner dropdown content
    • updates templ template to accept a parameter for showing/hiding the banner contents
  • updates "Here's how you know" clickable element:
    • replaces the <a> element with a <button> element for better keyboard navigability
  • updates styles to match the banner at https://www.cdc.gov/phin/php/vocabulary/

How to test:

  • Test at both full browser screen width (> 1000px) and smaller responsive widths ( < 900px)
  • Click "Here's how you know" and make sure the content properly shows/hides and styles do not change
  • Make sure you can interact with the clickable element with both a mouse/trackpad click, as well as tabbing to it and clicking with the space/enter key

Notes:

  • Keyboard navigability needs additional work; when using the keyboard to show/hide the banner content, focus should remain on the button element; currently you need to tab backwards to be able to focus and click it again. I'll add another ticket for this.

Feedback requested:

  • How can we use the Templ templating instead of creating additional HTML files to use with HTMX functionality? addressed in 99cbe18

@katyasoup katyasoup added the ready for review This issue or pull request is ready for feedback label Aug 29, 2024
@katyasoup
Copy link
Collaborator Author

@nickclyde updated d2aac37 per our convo!

@@ -425,9 +425,9 @@ func (app *Application) handleBannerToggle(w http.ResponseWriter, r *http.Reques
action := r.PathValue("action")
var component templ.Component
if action == "close" {
component = components.UsaBanner()
component = components.UsaBanner("close")
Copy link
Member

Choose a reason for hiding this comment

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

This looks great! You could simplify it even further by removing this if statement and just doing component = components.UsaBanner(action)!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

💡 done!

Copy link
Member

@nickclyde nickclyde left a comment

Choose a reason for hiding this comment

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

Looks great!!

@katyasoup katyasoup merged commit 80ba1db into main Sep 3, 2024
@katyasoup katyasoup deleted the kcd/usa-banner branch September 3, 2024 20:03
@akintner
Copy link
Collaborator

akintner commented Sep 3, 2024

LGTM 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This issue or pull request is ready for feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI: USA Banner + Dropdown template
3 participants