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

Lewis fletcher patch 1 #131

Conversation

LewisFletcher
Copy link

No description provided.

Added an 'id' to the select field to allow for htmx dynamic content using the id as a target.
@LewisFletcher
Copy link
Author

This will address the issue highlighted in #128

@@ -2,12 +2,12 @@
{% load l10n %}

<div class="relative">
<select class="{% if field.errors %}border border-red-500 {% endif %}bg-white focus:outline-none border border-gray-300 rounded-lg py-2 px-4 block w-full appearance-none leading-normal text-gray-700" name="{{ field.html_name }}" {{ field.field.widget.attrs|flatatt }}>
<select id= "{{ field.name }} class="{% if field.errors %}border border-red-500 {% endif %}bg-white focus:outline-none border border-gray-300 rounded-lg py-2 px-4 block w-full appearance-none leading-normal text-gray-700" name="{{ field.html_name }}" {{ field.field.widget.attrs|flatatt }}>
Copy link

Choose a reason for hiding this comment

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

this should read:

Suggested change
<select id= "{{ field.name }} class="{% if field.errors %}border border-red-500 {% endif %}bg-white focus:outline-none border border-gray-300 rounded-lg py-2 px-4 block w-full appearance-none leading-normal text-gray-700" name="{{ field.html_name }}" {{ field.field.widget.attrs|flatatt }}>
<select id="{{ field.name }}" class="{% if field.errors %}border border-red-500 {% endif %}bg-white focus:outline-none border border-gray-300 rounded-lg py-2 px-4 block w-full appearance-none leading-normal text-gray-700" name="{{ field.html_name }}" {{ field.field.widget.attrs|flatatt }}>

but even with the format fixed this would only be valid if the field name is unique for the whole page, and if it follows the rules for HTML IDs

Copy link

Choose a reason for hiding this comment

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

using field.html_name would probably be correct (it’s used for the name attribute, which serves a different role than the id attribute)

@GitRon
Copy link
Contributor

GitRon commented Jan 5, 2024

Hi @LewisFletcher

did you see the review remarks from @merwok?

Best
Ronny

@LewisFletcher
Copy link
Author

Yes, thanks for the call out. I'll address these and push a new patch.

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