max / makeover-webview
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
1 file changed,
+89 insertions,
-8 deletions
| @@ -110,6 +110,18 @@ | |||
| 110 | 110 | pub placeholder: Option<&'a str>, | |
| 111 | 111 | /// Markup appended inside the group, after the hint. Not escaped. | |
| 112 | 112 | pub trailing: Option<Markup<'a>>, | |
| 113 | + | /// Scopes the `id` attributes to one instance of the form. | |
| 114 | + | /// | |
| 115 | + | /// The field's `name` is what the value submits under and is the same | |
| 116 | + | /// wherever the form appears; its `id` has to be unique in the document, | |
| 117 | + | /// and those two facts stop agreeing the moment a form appears twice. | |
| 118 | + | /// goingson hits this directly: its new-task and edit-task modals are the | |
| 119 | + | /// same field set, so it prefixes `form-modal-task-new` or `-edit` to keep | |
| 120 | + | /// `label for` and `aria-describedby` pointing at the right control. | |
| 121 | + | /// | |
| 122 | + | /// Applies to `id`, `for` and the `-hint` / `-error` associations. Never to | |
| 123 | + | /// `name`, which would change what the form submits. | |
| 124 | + | pub id_prefix: Option<&'a str>, | |
| 113 | 125 | } | |
| 114 | 126 | ||
| 115 | 127 | impl<'a> Filling<'a> { | |
| @@ -120,6 +132,15 @@ | |||
| 120 | 132 | value, | |
| 121 | 133 | placeholder: None, | |
| 122 | 134 | trailing: None, | |
| 135 | + | id_prefix: None, | |
| 136 | + | } | |
| 137 | + | } | |
| 138 | + | ||
| 139 | + | /// The document-unique id for a field of this name. | |
| 140 | + | fn id_for(&self, name: &str) -> String { | |
| 141 | + | match self.id_prefix { | |
| 142 | + | Some(prefix) => format!("{}-{}", escape(prefix), escape(name)), | |
| 143 | + | None => escape(name), | |
| 123 | 144 | } | |
| 124 | 145 | } | |
| 125 | 146 | } | |
| @@ -167,8 +188,12 @@ | |||
| 167 | 188 | /// as if nothing were wrong. goingson's runtime validation path sets the | |
| 168 | 189 | /// attribute and its initial render does not, which is exactly the drift one | |
| 169 | 190 | /// emitter removes. | |
| 170 | - | fn control_attributes(field: &Field<'_>, id: &str) -> String { | |
| 171 | - | let mut attrs = format!(" id=\"{0}\" name=\"{0}\"", escape(id)); | |
| 191 | + | /// `id` and `name` arrive separately because they are not the same fact. The | |
| 192 | + | /// name is what submits and is fixed by the description; the id has to be | |
| 193 | + | /// unique in the document and so carries [`Filling::id_prefix`] when a form | |
| 194 | + | /// appears more than once. | |
| 195 | + | fn control_attributes(field: &Field<'_>, id: &str, name: &str) -> String { | |
| 196 | + | let mut attrs = format!(" id=\"{id}\" name=\"{}\"", escape(name)); | |
| 172 | 197 | if field.required { | |
| 173 | 198 | attrs.push_str(" required"); | |
| 174 | 199 | } | |
| @@ -182,10 +207,10 @@ | |||
| 182 | 207 | // place; naming both here means the hint survives an error appearing. | |
| 183 | 208 | let mut described = Vec::new(); | |
| 184 | 209 | if field.hint.is_some() { | |
| 185 | - | described.push(format!("{}-hint", escape(id))); | |
| 210 | + | described.push(format!("{id}-hint")); | |
| 186 | 211 | } | |
| 187 | 212 | if field.error.is_some() { | |
| 188 | - | described.push(format!("{}-error", escape(id))); | |
| 213 | + | described.push(format!("{id}-error")); | |
| 189 | 214 | } | |
| 190 | 215 | if !described.is_empty() { | |
| 191 | 216 | let _ = write!(attrs, " aria-describedby=\"{}\"", described.join(" ")); | |
| @@ -223,8 +248,8 @@ | |||
| 223 | 248 | ||
| 224 | 249 | /// The control itself, without its label, hint or error. | |
| 225 | 250 | fn control_html(field: &Field<'_>, filling: &Filling<'_>, opts: &Emit) -> String { | |
| 226 | - | let id = field.name; | |
| 227 | - | let attrs = control_attributes(field, id); | |
| 251 | + | let id = filling.id_for(field.name); | |
| 252 | + | let attrs = control_attributes(field, &id, field.name); | |
| 228 | 253 | let field_class = class("field", opts); | |
| 229 | 254 | let placeholder = filling.placeholder.map_or_else(String::new, |text| { | |
| 230 | 255 | format!(" placeholder=\"{}\"", escape(text)) | |
| @@ -302,11 +327,14 @@ | |||
| 302 | 327 | /// ``` | |
| 303 | 328 | #[must_use] | |
| 304 | 329 | pub fn field_html(field: &Field<'_>, filling: &Filling<'_>, opts: &Emit) -> String { | |
| 305 | - | let id = escape(field.name); | |
| 330 | + | let id = filling.id_for(field.name); | |
| 306 | 331 | ||
| 307 | 332 | if !field.kind.visible() { | |
| 333 | + | // Name only, no id: a hidden field is never pointed at by a label or a | |
| 334 | + | // description, so the one attribute it needs is the one that submits. | |
| 308 | 335 | return format!( | |
| 309 | - | "<input type=\"hidden\" name=\"{id}\" value=\"{}\">", | |
| 336 | + | "<input type=\"hidden\" name=\"{}\" value=\"{}\">", | |
| 337 | + | escape(field.name), | |
| 310 | 338 | escape(filling.value.as_text()) | |
| 311 | 339 | ); | |
| 312 | 340 | } | |
| @@ -522,4 +550,57 @@ | |||
| 522 | 550 | let html = field_html(&f, &Filling::default(), &Emit::default()); | |
| 523 | 551 | assert!(html.contains("data-extended=\"true\""), "{html}"); | |
| 524 | 552 | } | |
| 553 | + | ||
| 554 | + | /// The prefix scopes the id and leaves the name alone. Prefixing the name | |
| 555 | + | /// too would change what the form submits, which is the failure this pair | |
| 556 | + | /// of assertions exists to catch rather than describe. | |
| 557 | + | #[test] | |
| 558 | + | fn the_id_prefix_scopes_the_id_and_never_the_name() { | |
| 559 | + | let mut f = field(FieldKind::Text); | |
| 560 | + | f.hint = Some("Keep it short"); | |
| 561 | + | f.error = Some("Required"); | |
| 562 | + | let filling = Filling { | |
| 563 | + | id_prefix: Some("form-modal-task-edit"), | |
| 564 | + | ..Filling::default() | |
| 565 | + | }; | |
| 566 | + | let html = field_html(&f, &filling, &Emit::default()); | |
| 567 | + | ||
| 568 | + | assert!(html.contains(r#"id="form-modal-task-edit-title""#), "{html}"); | |
| 569 | + | assert!(html.contains(r#"name="title""#), "{html}"); | |
| 570 | + | assert!(!html.contains(r#"name="form-modal-task-edit-title""#), "{html}"); | |
| 571 | + | ||
| 572 | + | // The label and both associations follow the id, or they point at | |
| 573 | + | // nothing once the same form is on screen twice. | |
| 574 | + | assert!( | |
| 575 | + | html.contains(r#"for="form-modal-task-edit-title""#), | |
| 576 | + | "{html}" | |
| 577 | + | ); | |
| 578 | + | assert!( | |
| 579 | + | html.contains( | |
| 580 | + | r#"aria-describedby="form-modal-task-edit-title-hint form-modal-task-edit-title-error""# | |
| 581 | + | ), | |
| 582 | + | "{html}" | |
| 583 | + | ); | |
| 584 | + | assert!( | |
| 585 | + | html.contains(r#"id="form-modal-task-edit-title-hint""#), | |
| 586 | + | "{html}" | |
| 587 | + | ); | |
| 588 | + | } | |
| 589 | + | ||
| 590 | + | #[test] | |
| 591 | + | fn a_hidden_field_submits_its_bare_name_under_a_prefix() { | |
| 592 | + | let filling = Filling { | |
| 593 | + | value: Value::Text("42"), | |
| 594 | + | id_prefix: Some("scoped"), | |
| 595 | + | ..Filling::default() | |
| 596 | + | }; | |
| 597 | + | let html = field_html(&field(FieldKind::Hidden), &filling, &Emit::default()); | |
| 598 | + | assert_eq!(html, r#"<input type="hidden" name="title" value="42">"#); | |
| 599 | + | } | |
| 600 | + | ||
| 601 | + | #[test] | |
| 602 | + | fn no_prefix_leaves_the_id_as_the_name() { | |
| 603 | + | let html = field_html(&field(FieldKind::Text), &Filling::default(), &Emit::default()); | |
| 604 | + | assert!(html.contains(r#"id="title" name="title""#), "{html}"); | |
| 605 | + | } | |
| 525 | 606 | } |