-
Notifications
You must be signed in to change notification settings - Fork 128
Closed
Labels
Description
Steps to reproduce
- Create a TextField with
show_clear_buttonset to true, but without aclear_button_id
Lookbook example
https://primer.style/view-components/lookbook/inspect/primer/alpha/text_field/playground?show_clear_button=true&clear_button_id=
Code example
render(Primer::Alpha::TextField.new(
name: :some_name,
id: some_id,
label: "My text field",
show_clear_button: true,
))
Actual behaviour
- The clear button is rendered with an empy id.
Expected
- If no ID is provided, the component should generate one (like it is done for most other components)
Why is this a problem?
- It is against the W3C definition for
id
ID and NAME tokens must begin with a letter ([A-Za-z]) and may be followed by any number of letters, digits ([0-9]), hyphens ("-"), underscores ("_"), colons (":"), and periods (".").
(Source: Basic HTML data types)
- Turbo will fail to morph a response if there are elements like on the page and in the response. It is an issue with Idiomorph's id map logic. Idiomorph will collect the empty ids in an id map, and that is being used to match elements faster on the page. However, when the response comes with an empty id, Idiomorph will try to find it on the page, which will raise a javascript error: Uncaught DOMException: ctx.target.querySelector: '#' is not a valid selector.
This is a bug in Idiomorph, but as a sidenote, primer could output the text field's clear button tag with an empty id="", if a clear_button_id argument is not explicitly provided.
As a bottom line, when a text field has a close button rendered, it must always have an id, otherwise it turbo will fail to morph the response.