rusackas commented on code in PR #32226:
URL: https://github.com/apache/superset/pull/32226#discussion_r1953221595
##########
superset-frontend/src/dashboard/components/gridComponents/Markdown.test.jsx:
##########
@@ -47,134 +33,325 @@ describe('Markdown', () => {
editMode: false,
availableColumnCount: 12,
columnWidth: 50,
- redoLength: 0,
- undoLength: 0,
- onResizeStart() {},
- onResize() {},
- onResizeStop() {},
- handleComponentDrop() {},
- updateComponents() {},
- deleteComponent() {},
- logEvent() {},
- addDangerToast() {},
+ onResizeStart: jest.fn(),
+ onResize: jest.fn(),
+ onResizeStop: jest.fn(),
+ handleComponentDrop: jest.fn(),
+ updateComponents: jest.fn(),
+ deleteComponent: jest.fn(),
+ logEvent: jest.fn(),
+ addDangerToast: jest.fn(),
};
- function setup(overrideProps) {
- // We have to wrap provide DragDropContext for the underlying DragDroppable
- // otherwise we cannot assert on Droppable children
- const wrapper = mount(
- <Provider store={mockStore}>
- <DndProvider backend={HTML5Backend}>
- <MarkdownConnected {...props} {...overrideProps} />
- </DndProvider>
- </Provider>,
- );
- return wrapper;
- }
+ beforeAll(() => {
+ jest.spyOn(console, 'error').mockImplementation(msg => {
+ if (
+ typeof msg === 'string' &&
+ !msg.includes('[antd:') &&
+ !msg.includes('Warning: An update to SafeMarkdown') &&
+ !msg.includes('Warning: React does not recognize') &&
+ !msg.includes("Warning: Can't perform a React state update")
+ ) {
+ console.error(msg);
+ }
+ });
+ });
- it('should render a Draggable', () => {
- const wrapper = setup();
- expect(wrapper.find(Draggable)).toBeTruthy();
+ afterAll(() => {
+ jest.restoreAllMocks();
});
- it('should render a WithPopoverMenu', () => {
- const wrapper = setup();
- expect(wrapper.find(WithPopoverMenu)).toBeTruthy();
+ afterEach(() => {
+ jest.clearAllMocks();
});
- it('should render a ResizableContainer', () => {
- const wrapper = setup();
- expect(wrapper.find(ResizableContainer)).toBeTruthy();
+ const setup = async (overrideProps = {}) => {
+ let utils;
+ await act(async () => {
+ utils = render(<MarkdownConnected {...props} {...overrideProps} />, {
+ useDnd: true,
+ store: mockStore,
+ });
+ await new Promise(resolve => setTimeout(resolve, 0));
+ });
+ return utils;
+ };
+
+ afterEach(() => {
+ jest.clearAllMocks();
});
- it('should only have an adjustableWidth if its parent is a Row', () => {
- let wrapper = setup();
-
expect(wrapper.find(ResizableContainer).prop('adjustableWidth')).toBe(true);
+ it('should render the markdown component', async () => {
+ await setup();
+
expect(screen.getByTestId('dashboard-markdown-editor')).toBeInTheDocument();
+ });
- wrapper = setup({ ...props, parentComponent: mockLayout.present.CHART_ID
});
- expect(wrapper.find(ResizableContainer).prop('adjustableWidth')).toBe(
- false,
- );
+ it('should render the markdown content in preview mode by default', async ()
=> {
+ await setup();
+ expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
+ expect(
+ screen.getByTestId('dashboard-component-chart-holder'),
+ ).toBeInTheDocument();
});
- it('should pass correct props to ResizableContainer', () => {
- const wrapper = setup();
- const resizableProps = wrapper.find(ResizableContainer).props();
- expect(resizableProps.widthStep).toBe(props.columnWidth);
- expect(resizableProps.widthMultiple).toBe(props.component.meta.width);
- expect(resizableProps.heightMultiple).toBe(props.component.meta.height);
- expect(resizableProps.maxWidthMultiple).toBe(
- props.component.meta.width + props.availableColumnCount,
- );
+ it('should render editor when in edit mode and clicked', async () => {
+ await setup({ editMode: true });
+ const container = screen.getByTestId('dashboard-component-chart-holder');
+ await act(async () => {
+ fireEvent.click(container);
+ });
+
+ expect(await screen.findByRole('textbox')).toBeInTheDocument();
});
- it('should render an Markdown when NOT focused', () => {
- const wrapper = setup();
- expect(wrapper.find(MarkdownEditor).length).toBe(0);
- expect(wrapper.find(SafeMarkdown).length).toBeGreaterThan(0);
+ it('should switch between edit and preview modes', async () => {
+ await setup({ editMode: true });
+ const container = screen.getByTestId('dashboard-component-chart-holder');
+
+ await act(async () => {
+ fireEvent.click(container);
+ });
+
+ expect(await screen.findByRole('textbox')).toBeInTheDocument();
+
+ // Find and click edit dropdown by role
+ const editButton = screen.getByRole('button', { name: /edit/i });
+ await act(async () => {
+ fireEvent.click(editButton);
+ });
+
+ // Click preview option in dropdown
+ const previewOption = await screen.findByText(/preview/i);
+ await act(async () => {
+ fireEvent.click(previewOption);
+ });
+
+ expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
+ });
+
+ it('should call updateComponents when switching from edit to preview with
changes', async () => {
+ const updateComponents = jest.fn();
+ const mockCode = 'new markdown!';
+
+ const { container } = await setup({
+ editMode: true,
+ updateComponents,
+ component: {
+ ...mockLayout.present.MARKDOWN_ID,
+ id: 'test',
+ meta: { code: '' },
+ },
+ });
+
+ // Enter edit mode and change content
+ await act(async () => {
+ const markdownHolder = screen.getByTestId(
+ 'dashboard-component-chart-holder',
+ );
+ fireEvent.click(markdownHolder);
+
+ // Wait for editor to be fully mounted
+ await new Promise(resolve => setTimeout(resolve, 50));
+
+ // Find the actual textarea/input element
+ const editor = container.querySelector('.ace_text-input');
+ console.log('Editor element:', editor);
+
+ // Simulate direct input
+ fireEvent.input(editor, { target: { value: mockCode } });
+ console.log('After input:', editor.value);
+
+ // Force blur and change events
+ fireEvent.change(editor, { target: { value: mockCode } });
+ fireEvent.blur(editor);
+
+ // Wait for state update
+ await new Promise(resolve => setTimeout(resolve, 50));
+
+ // Click the Edit dropdown button
+ const editDropdown = screen.getByText('Edit');
+ fireEvent.click(editDropdown);
+
+ // Wait for dropdown to open
+ await new Promise(resolve => setTimeout(resolve, 50));
+
+ // Find and click preview in dropdown
+ const previewOption = await screen.findByText(/preview/i);
+ fireEvent.click(previewOption);
+
+ // Wait for update to complete
+ await new Promise(resolve => setTimeout(resolve, 50));
+ });
+
+ console.log('Component state:', {
+ updateCalls: updateComponents.mock.calls,
+ editorVisible: screen.queryByRole('textbox') !== null,
+ dropdownOpen: screen.queryByText(/preview/i) !== null,
+ });
+
+ // Update assertion to match actual component structure
+ expect(updateComponents).toHaveBeenCalledWith({
+ test: {
+ id: 'test',
+ meta: { code: mockCode },
+ type: 'MARKDOWN',
+ children: [],
+ parents: [],
+ },
+ });
});
- it('should render an AceEditor when focused and editMode=true and
editorMode=edit', async () => {
- const wrapper = setup({ editMode: true });
- expect(wrapper.find(MarkdownEditor).length).toBe(0);
- expect(wrapper.find(SafeMarkdown).length).toBeGreaterThan(0);
- act(() => {
- wrapper.find(WithPopoverMenu).simulate('click'); // focus + edit
+ it('should show placeholder text when markdown is empty', async () => {
+ await setup({
+ component: {
+ ...mockLayout.present.MARKDOWN_ID,
+ meta: { code: '' },
+ },
});
- await waitForComponentToPaint(wrapper);
- expect(wrapper.find(MarkdownEditor).length).toBeGreaterThan(0);
- expect(wrapper.find(SafeMarkdown).length).toBe(0);
+
+ expect(
+ screen.getByText(/Click here to learn more about/),
+ ).toBeInTheDocument();
});
- it('should render a ReactMarkdown when focused and editMode=true and
editorMode=preview', () => {
- const wrapper = setup({ editMode: true });
- wrapper.find(WithPopoverMenu).simulate('click'); // focus + edit
- expect(wrapper.find(MarkdownEditor).length).toBeGreaterThan(0);
- expect(wrapper.find(SafeMarkdown).length).toBe(0);
+ it('should handle markdown errors gracefully', async () => {
+ const addDangerToast = jest.fn();
+ const { container } = await setup({
+ addDangerToast,
+ component: {
+ ...mockLayout.present.MARKDOWN_ID,
+ meta: { code: '# Test' },
+ },
+ });
- // we can't call setState on Markdown bc it's not the root component, so
call
- // the mode dropdown onchange instead
- const dropdown = wrapper.find(MarkdownModeDropdown);
- dropdown.prop('onChange')('preview');
- wrapper.update();
+ console.log('Component structure:', {
+ markdownEditor: screen.getByTestId('dashboard-markdown-editor'),
+ safeMarkdown: container.querySelector('.safe-markdown'),
+ allEventListeners: container.querySelectorAll('[data-test]'),
+ toastFn: addDangerToast.toString(),
+ });
+
+ await act(async () => {
+ const markdownEditor = screen.getByTestId('dashboard-markdown-editor');
+ ['error', 'markdownError', 'renderError'].forEach(eventType => {
+ const event = new CustomEvent(eventType, {
+ bubbles: true,
+ detail: { error: new Error('Markdown error') },
+ });
+ console.log(`Dispatching ${eventType} event`);
+ markdownEditor.dispatchEvent(event);
+ });
- expect(wrapper.find(SafeMarkdown).length).toBeGreaterThan(0);
- expect(wrapper.find(MarkdownEditor).length).toBe(0);
+ await new Promise(resolve => setTimeout(resolve, 100));
+ console.log('After events:', {
+ toastCalls: addDangerToast.mock.calls,
+ errorElements: container.querySelectorAll('.error-message'),
+ });
+ });
});
- it('should call updateComponents when editMode changes from edit => preview,
and there are markdownSource changes', () => {
- const updateComponents = sinon.spy();
- const wrapper = setup({ editMode: true, updateComponents });
- wrapper.find(WithPopoverMenu).simulate('click'); // focus + edit
-
- // we can't call setState on Markdown bc it's not the root component, so
call
- // the mode dropdown onchange instead
- const dropdown = wrapper.find(MarkdownModeDropdown);
- dropdown.prop('onChange')('preview');
- expect(updateComponents.callCount).toBe(0);
-
- dropdown.prop('onChange')('edit');
- // because we can't call setState on Markdown, change it through the editor
- // then go back to preview mode to invoke updateComponents
- const editor = wrapper.find(MarkdownEditor);
- editor.prop('onChange')('new markdown!');
- dropdown.prop('onChange')('preview');
- expect(updateComponents.callCount).toBe(1);
+ it('should resize editor when width changes', async () => {
+ const { container, rerender } = await setup({ editMode: true });
+
+ await act(async () => {
+ const chartHolder = screen.getByTestId(
+ 'dashboard-component-chart-holder',
+ );
+ fireEvent.click(chartHolder);
+ await new Promise(resolve => setTimeout(resolve, 50));
+ });
+
+ const editorContainer = screen.getByTestId('dashboard-markdown-editor');
+ console.log('Initial component state:', {
+ container: {
+ width: container.offsetWidth,
+ style: window.getComputedStyle(container),
+ },
+ editor: {
+ width: editorContainer.offsetWidth,
+ style: window.getComputedStyle(editorContainer),
+ children: editorContainer.children.length,
+ },
+ aceEditor: container.querySelector('.ace_editor'),
+ });
Review Comment:
Huh... i thought linting would have caught that! Nice job on these, Mr.
Eagle-eyes!
--
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: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
