bito-code-review[bot] commented on code in PR #44858:
URL: https://github.com/apache/superset/pull/44858#discussion_r4160659280
##########
superset-frontend/src/explore/components/controls/TextAreaControl.test.tsx:
##########
@@ -61,4 +62,143 @@ 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={() => {}}
+ />
+ </>
+ );
+ }
Review Comment:
<!-- Bito Reply -->
The update to use a shared SyncWrapper is a clean approach that addresses
the redundancy identified in the review. This change simplifies the test
structure and ensures that future modifications to the component props only
need to be applied in one location.
--
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]
