sadpandajoe commented on code in PR #44452:
URL: https://github.com/apache/superset/pull/44452#discussion_r4236260013


##########
superset-frontend/src/explore/components/controls/CheckboxControl.tsx:
##########
@@ -50,24 +58,77 @@ const CheckBoxControlWrapper = styled.div`
 export default function CheckboxControl({
   value = false,
   label,
+  disabled = false,
+  disabledReason,
+  resetLabel,
   onChange = () => {},
   ...restProps
 }: CheckboxControlProps): JSX.Element {
+  const explanationId = useId();
+  const generatedCheckboxId = useId();
+  const checkboxId = restProps.name || generatedCheckboxId;

Review Comment:
   Now that the checkbox input gets `id={name}`, controls that reuse a static 
`name` across several mounted instances end up with duplicate DOM ids. The 
annotation layer editors do this (`annotation-layer-show-markers`, 
`annotation-layer-hide-line`, `annotation-override-time_range`, ...), and 
`ControlPopover` leaves closed popovers mounted (`destroyOnHidden` defaults to 
false). After opening layer A and then layer B, B's header label `htmlFor` 
resolves to A's hidden input, and the new 
`document.getElementById(checkboxId)?.focus()` on label click also lands on A's 
input, so B's checkbox ends up with the wrong accessible-name association and 
focus goes to a hidden element. Should the input id be scoped per instance (for 
example by always using the generated id for the input and only forwarding 
`name` for the `data-test` attributes)?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to