Skip to content

Add more form attributes#352

Open
ElijahAhianyo wants to merge 11 commits intocot-rs:masterfrom
ElijahAhianyo:elijah/more-form-attrs
Open

Add more form attributes#352
ElijahAhianyo wants to merge 11 commits intocot-rs:masterfrom
ElijahAhianyo:elijah/more-form-attrs

Conversation

@ElijahAhianyo
Copy link
Contributor

@ElijahAhianyo ElijahAhianyo commented Jun 18, 2025

This PR adds more form attributes to various HTML elements in cot

@github-actions github-actions bot added the C-lib Crate: cot (main library crate) label Jun 18, 2025
@codecov
Copy link

codecov bot commented Jun 18, 2025

Codecov Report

❌ Patch coverage is 92.98701% with 27 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cot/src/form/fields.rs 90.26% 5 Missing and 21 partials ⚠️
cot/src/form/fields/files.rs 93.75% 0 Missing and 1 partial ⚠️
Flag Coverage Δ
rust 89.75% <92.98%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cot-core/src/html.rs 97.65% <100.00%> (+0.20%) ⬆️
cot/src/form/fields/attrs.rs 100.00% <100.00%> (+100.00%) ⬆️
cot/src/form/fields/files.rs 93.08% <93.75%> (-0.07%) ⬇️
cot/src/form/fields.rs 89.10% <90.26%> (+0.85%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ElijahAhianyo ElijahAhianyo force-pushed the elijah/more-form-attrs branch from 4824f19 to e6b2f3d Compare July 12, 2025 18:20
@ElijahAhianyo ElijahAhianyo marked this pull request as ready for review February 8, 2026 20:05
@ElijahAhianyo ElijahAhianyo force-pushed the elijah/more-form-attrs branch from c55425b to c7ba160 Compare February 8, 2026 21:34
@ElijahAhianyo ElijahAhianyo changed the title wip: Add more form attributes Add more form attributes Feb 8, 2026
@ElijahAhianyo ElijahAhianyo requested review from m4tx and seqre February 9, 2026 11:44
/// let rendered = data_list.render();
/// ```
#[must_use]
pub fn data_list<I, S>(list: I, id: &str) -> Self
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if we should force providing id in the constructor, maybe getting rid of it and expecting users to do .attr("id", id") is enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mm, The thing about the datalist element is that the id attr is tightly coupled with the list attribute in the input element (i.e the list attr in the input should match the id attr in the datalist) so if the user forgets to call .attr("id", id) it wont work. I had that as a parameter because it's required for correctness

Comment on lines +89 to +91
pub autocapitalize: Option<AutoCapitalize>,
/// Corresponds to the [`AutoComplete`] attribute in the HTML input element.
pub autocomplete: Option<AutoComplete>,
Copy link
Member

Choose a reason for hiding this comment

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

Those are global attributes so they apply to all HTML elements, maybe we should add some macro that will add them to every HTML element struct we create or via composition? What do you think @m4tx? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Attributes/autocomplete

The autocomplete attr is actually not a global element, so I'm not sure all form fields should have it.
For attributes generic to all form elements, it looks like we have the FormFieldOptions, which has a few attributes common to all elements. Will that not work?

It is available on elements that take a text or numeric value as input, <textarea> elements, elements, and elements.

Copy link
Member

Choose a reason for hiding this comment

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

My bad, I selected clumsily the lines for the comment. autocomplete is not a global attribute, but if you check the MDN docs, autocapitalize and dir are. Those should be added to HTML tag's in an uniform and global way.

While you're right that FormFieldOptions are something like that, those are limited only to form fields and contain name and required which are not global attributes.

I suppose the best way would be to implement that into HtmlTag, but that would be out of scope for this PR.

impl HtmlSafe for EmailField {}

impl_form_field!(IntegerField, IntegerFieldOptions, "an integer", T: Integer);
impl_form_field!(IntegerField, IntegerFieldOptions, "an integer", T: Integer + Display);
Copy link
Member

Choose a reason for hiding this comment

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

Why was it needed here and below for float field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think enum attributes like Step typically provide variants like Any and Value(T). When converting the step value to a string we need T to implement Display to convert them into strings, and since integers and floats now have the step attribute, the typing ripples up

///
/// [`list`]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#list
#[derive(Debug, Clone, Default)]
pub struct List(Vec<String>);
Copy link
Member

Choose a reason for hiding this comment

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

The linked docs about list attribute for <input> claim that:

The value given to the list attribute should be the id of a element located in the same document.

I understand that your approach was of simplicity, but I think we should first and foremost support the spec and then add conveniences for the users.

I'd say that new should take the id of datalist by default. Or even better, take a datalist struct, but now it's generic HtmlTag, which doesn't make it that much better.

Then, we can provide a convenience constructor, let's say from_list, that has the existing functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if I fully understand your suggestion. The List type is used to add data-list options to the input element, so the way i thought of it was, you have an input field that has an id and if you want to specify a data-list for that input type, you can pass in a list of option values as the list attribute value then we implicitly create the data list element with the values from the list as option.

#[derive(Debug, Form)]
struct ExampleForm {
  #[form(
        opts(
           list=List::new(["Title A", "Title B", "Title C"])
        )
    )]
  title,
  ...
}

Which should render into:

<div>
    <input type="text" id="title" list="__title_datalist">
    <datalist id="__test_datalist">
        <option value="Title A"/>
        <option value="Title B"/>
        <option value="Titile C"/>
    </datalist>
</div>

The Id lives on the input element and not on the list because the list attribute (which later gets expanded into the datalist element) cant operate independently unless coupled with the input element. Also, keeping the API this way makes it consistent with the signature of other attribute types

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-lib Crate: cot (main library crate)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments