Skip to content

Commit

Permalink
[Dialog] Log error instead of throwing for missing titles (#2948)
Browse files Browse the repository at this point in the history
* Replace Error with console.error

* Track versions
  • Loading branch information
vladmoroz authored Jun 11, 2024
1 parent d716c48 commit ed65bf9
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 5 deletions.
6 changes: 6 additions & 0 deletions .yarn/versions/261663d9.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
releases:
"@radix-ui/react-alert-dialog": patch
"@radix-ui/react-dialog": patch

declined:
- primitives
19 changes: 15 additions & 4 deletions packages/react/dialog/src/Dialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,17 @@ describe('given a default Dialog', () => {
let closeButton: HTMLElement;
let consoleWarnMock: jest.SpyInstance;
let consoleWarnMockFunction: jest.Mock;
let consoleErrorMock: jest.SpyInstance;
let consoleErrorMockFunction: jest.Mock;

beforeEach(() => {
// This surpresses React error boundary logs for testing intentionally
// thrown errors, like in some test cases in this suite. See discussion of
// this here: https://github.com/facebook/react/issues/11098
consoleWarnMockFunction = jest.fn();
consoleWarnMock = jest.spyOn(console, 'warn').mockImplementation(consoleWarnMockFunction);
consoleErrorMockFunction = jest.fn();
consoleErrorMock = jest.spyOn(console, 'error').mockImplementation(consoleErrorMockFunction);

rendered = render(<DialogTest />);
trigger = rendered.getByText(OPEN_TEXT);
Expand All @@ -64,6 +68,8 @@ describe('given a default Dialog', () => {
afterEach(() => {
consoleWarnMock.mockRestore();
consoleWarnMockFunction.mockClear();
consoleErrorMock.mockRestore();
consoleErrorMockFunction.mockClear();
});

it('should have no accessibility violations in default state', async () => {
Expand All @@ -83,10 +89,15 @@ describe('given a default Dialog', () => {
});

describe('when no title has been provided', () => {
it('should throw an error', () =>
expect(() => {
renderAndClickDialogTrigger(<NoLabelDialogTest />);
}).toThrowError());
beforeEach(() => {
cleanup();
});
it('should display an error in the console', () => {
consoleErrorMockFunction.mockClear();

renderAndClickDialogTrigger(<NoLabelDialogTest />);
expect(consoleErrorMockFunction).toHaveBeenCalled();
});
});

describe('when aria-describedby is set to undefined', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/react/dialog/src/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ For more information, see https://radix-ui.com/primitives/docs/components/${titl
React.useEffect(() => {
if (titleId) {
const hasTitle = document.getElementById(titleId);
if (!hasTitle) throw new Error(MESSAGE);
if (!hasTitle) console.error(MESSAGE);
}
}, [MESSAGE, titleId]);

Expand Down

0 comments on commit ed65bf9

Please sign in to comment.