-
Notifications
You must be signed in to change notification settings - Fork 0
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
Conversation
@nickclyde updated d2aac37 per our convo! |
internal/app/handlers.go
Outdated
@@ -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") |
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.
This looks great! You could simplify it even further by removing this if statement and just doing component = components.UsaBanner(action)
!
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.
💡 done!
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.
Looks great!!
LGTM 🚀 |
resolves #33
What this PR changes:
htmx
swapping for show/hide functionality on the USA banner dropdown content<a>
element with a<button>
element for better keyboard navigabilityHow to test:
Notes:
Feedback requested:
How can we use theaddressed in 99cbe18Templ
templating instead of creating additional HTML files to use with HTMX functionality?