Fix "node.contains is not a function" crash on Modal/Popup triggers
Some checks are pending
CodeQL / Analyze (push) Waiting to run
Controller Tests / controller-tests (push) Waiting to run
Jest Tests / jest-tests (push) Waiting to run

The themed Button forwards its ref straight to SButton, a class
component, so the ref resolves to the class instance instead of the DOM
node. Semantic UI's Modal/Popup Portal calls node.contains(...) on the
trigger's ref during handleDocumentClick, which throws on a non-DOM
value. This crashed the Control page whenever a Service Logs (or QR /
download-error) modal was open and the document was clicked.

Wrap SButton in Ref (findDOMNode) so a forwarded ref always resolves to
the underlying <button> element, fixing every Modal/Popup that uses a
themed Button as its trigger.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Roland 2026-07-20 11:38:17 -06:00
parent b3dcb0046e
commit 6786e345a8
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
2 changed files with 37 additions and 2 deletions

View file

@ -1,5 +1,5 @@
import React from 'react';
import {Progress} from './Theme';
import {Button, Modal, Progress} from './Theme';
import {renderInDarkMode, renderInLightMode} from '../test-utils';
// All semantic-ui colors.
@ -10,6 +10,34 @@ const lightBarColors = ['yellow', 'olive', 'teal'];
// Bar colors that need dark percent text only in dark mode (inverted makes bar lighter).
const darkModeLightBarColors = ['grey', 'blue'];
describe('Button ref forwarding', () => {
// Regression: SButton is a class component, so forwarding a ref straight to it resolves to the class instance
// (no `.contains`). Semantic UI's Modal/Popup Portal calls `node.contains(...)` on the trigger's ref during
// handleDocumentClick, throwing "node.contains is not a function". The forwarded ref must be the DOM <button>.
it('resolves a forwarded ref to the underlying DOM element', () => {
const ref = React.createRef();
renderInLightMode(<Button ref={ref} content='Click'/>);
expect(ref.current).toBeInstanceOf(HTMLElement);
expect(typeof ref.current.contains).toBe('function');
expect(ref.current.tagName).toBe('BUTTON');
});
it('does not throw when a Button is used as a controlled Modal trigger and the document is clicked', () => {
// Reproduces the Control page crash: an open Modal whose trigger is a themed Button. A document click
// invokes the Portal's handleDocumentClick -> doesNodeContainClick(triggerRef.current, e).
renderInLightMode(
<Modal open trigger={<Button content='Open'/>}>
<Modal.Content>content</Modal.Content>
</Modal>
);
expect(() => {
document.body.dispatchEvent(new MouseEvent('click', {bubbles: true}));
}).not.toThrow();
});
});
describe('Progress component theme support', () => {
describe('dark mode', () => {
it('adds inverted-progress-text class for label readability', () => {

View file

@ -34,6 +34,7 @@ import {
Placeholder as SPlaceholder,
Popup as SPopup,
Progress as SProgress,
Ref as SRef,
Segment as SSegment,
Statistic as SStatistic,
StatisticGroup as SStatisticGroup,
@ -287,7 +288,13 @@ const ButtonGroup: ButtonGroupComponent = (props) => {
const ButtonBase = forwardRef<any, ButtonProps>((props, ref) => {
const {i, inverted} = useContext(ThemeContext);
const mergedProps = defaultGrey({...i, ...props}, !!inverted);
return <SButton ref={ref} {...mergedProps}/>
// SButton is a class component, so a forwarded `ref` would resolve to the class instance rather than the DOM
// node. Semantic UI's Modal/Popup use the trigger's ref with `node.contains(...)`, which throws on a non-DOM
// value. Wrap in Ref (findDOMNode) so the forwarded ref always resolves to the underlying <button> element.
if (ref) {
return <SRef innerRef={ref}><SButton {...mergedProps}/></SRef>
}
return <SButton {...mergedProps}/>
});
ButtonBase.displayName = 'ButtonBase';