ocsigen / tyxml

Build valid HTML and SVG documents
https://ocsigen.org/tyxml/
Other
163 stars 57 forks source link

Extend `autocomplete` attribute values #302

Closed aronerben closed 9 months ago

aronerben commented 2 years ago

Hello, thanks for this fantastic project!

Currently, the autocomplete attribute only supports on and off. The spec lists many more options. It would be nice if Tyxml supported those too! I don't know if this is already on the roadmap, or similar, so I decided to open this issue. :)

Encoding all possibilities mentioned in the spec with types seems a bit tedious (as there seem to be named groups, special cases for hidden input fields, etc.). Maybe simply turning it into a string attribute is "good enough" (I don't know what Tyxml's philosophy is here).

Currently we've solved it by unsafely adding the attribute like so:

let open Tyxml_xml in
let open Tyxml_html in
street_input
|> toelt
|> amap1 (fun _ attrs ->
      add_string_attrib "autocomplete" "street_address" attrs)
|> tot)

where street_input is some input field.

Drup commented 2 years ago

That specification is indeed pretty awkward, but I would prefer to avoid "just using a string". In this case, it seems we could start with a fairly simple model such as `On | `Off | `Tokens of string list and see where that leads us. In any case the ppx extension can check things in more details.

If you are willing, don't hesitate to submit a (potentially WIP) implementation ! There are instructions on how to add/change attributes in https://github.com/ocsigen/tyxml/blob/master/docs/Contributing.md . :)

aronerben commented 2 years ago

@Drup Thanks for the quick response! I've drafted a PR: https://github.com/ocsigen/tyxml/pull/303

Drup commented 9 months ago

Merged !