bito-code-review[bot] commented on code in PR #44858:
URL: https://github.com/apache/superset/pull/44858#discussion_r4156906370
##########
superset-frontend/src/explore/components/controls/TextAreaControl.test.tsx:
##########
@@ -61,4 +62,139 @@ describe('TextArea', () => {
fireEvent.change(textArea, { target: { value: 'x' } });
expect(defaultProps.onChange).toHaveBeenCalledWith('x');
});
+
+ test('picks up an externally changed initialValue without remounting the
AceEditor', async () => {
+ function Wrapper() {
+ const [initialValue, setInitialValue] = useState('first');
+ return (
+ <>
+ <button
+ type="button"
+ onClick={() => setInitialValue('synced-from-source')}
+ >
+ sync
+ </button>
+ <TextAreaControl
+ name="expr"
+ language="sql"
+ initialValue={initialValue}
+ onChange={() => {}}
+ />
+ </>
+ );
+ }
+ const { container } = render(<Wrapper />);
+ await waitFor(() => {
+ expect(container.querySelector('.ace_text-input')).toBeInTheDocument();
+ });
+ // react-ace keeps a reference to the live editor instance on the
+ // container node's `env`, which lets us read the underlying Ace
+ // document directly; jsdom doesn't paint Ace's text layer, so the
+ // rendered DOM has no visible text to assert against.
+ const editorNode = container.querySelector('.ace_editor') as HTMLElement &
{
+ env: { editor: { getValue: () => string } };
+ };
+ expect(editorNode.env.editor.getValue()).toBe('first');
+
+ fireEvent.click(screen.getByText('sync'));
+
+ await waitFor(() => {
+ expect(editorNode.env.editor.getValue()).toBe('synced-from-source');
+ });
+ // Same editor instance picked up the new value; it was not remounted.
+ expect(container.querySelector('.ace_editor')).toBe(editorNode);
+ });
+
+ test('does not re-notify onChange when an external sync sets the editor
value', async () => {
+ const onChange = jest.fn();
+ function Wrapper() {
+ const [initialValue, setInitialValue] = useState('first');
+ return (
+ <>
+ <button
+ type="button"
+ onClick={() => setInitialValue('synced-from-source')}
+ >
+ sync
+ </button>
+ <TextAreaControl
+ name="expr"
+ language="sql"
+ initialValue={initialValue}
+ onChange={onChange}
+ />
+ </>
+ );
+ }
+ const { container } = render(<Wrapper />);
+ await waitFor(() => {
+ expect(container.querySelector('.ace_text-input')).toBeInTheDocument();
+ });
+ const editorNode = container.querySelector('.ace_editor') as HTMLElement &
{
+ env: { editor: { getValue: () => string } };
+ };
+
+ fireEvent.click(screen.getByText('sync'));
+
+ await waitFor(() => {
+ expect(editorNode.env.editor.getValue()).toBe('synced-from-source');
+ });
+ // Ace's setValue() fires its own change event; that echo must not be
+ // mistaken for the user typing and bounced back up through onChange.
+ expect(onChange).not.toHaveBeenCalled();
+ });
+
+ test('keeps syncing the inline editor after the edit-in-modal dialog is
opened and closed', async () => {
+ function Wrapper() {
+ const [initialValue, setInitialValue] = useState('first');
+ return (
+ <>
+ <button
+ type="button"
+ onClick={() => setInitialValue('synced-from-source')}
+ >
+ sync
+ </button>
+ <TextAreaControl
+ name="expr"
+ language="sql"
+ initialValue={initialValue}
+ onChange={() => {}}
+ />
+ </>
+ );
+ }
+ const { container } = render(<Wrapper />);
+ await waitFor(() => {
+ expect(container.querySelector('.ace_text-input')).toBeInTheDocument();
+ });
+ const inlineEditorNode = container.querySelector(
+ '.ace_editor',
+ ) as HTMLElement & {
+ env: { editor: { getValue: () => string } };
+ };
+
+ // Open, then close, the "edit in modal" dialog. Its own Ace instance
+ // mounts and is destroyed (destroyOnHidden) in the process; this used
+ // to leave the single shared editor ref pointing at that destroyed
+ // instance instead of the still-visible inline editor.
+ // ModalTrigger wraps its triggerNode in its own role="button" element,
+ // so a <Button> triggerNode (as used here) yields two nested elements
+ // matching role "button" with the same accessible name; target the
+ // wrapper directly via its data-test id instead.
+ fireEvent.click(screen.getByTestId('span-modal-trigger'));
+ await waitFor(() => {
+ expect(container.querySelectorAll('.ace_editor')).toHaveLength(2);
+ });
Review Comment:
<!-- Bito Reply -->
The change to query `document` instead of `container` is appropriate. Since
the modal is rendered via a portal to `document.body`, it exists outside the
standard React Testing Library `container` node, making it inaccessible to
queries scoped to that container. This adjustment ensures the test correctly
detects the modal's editor instance.
--
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]
