rusackas commented on code in PR #41285:
URL: https://github.com/apache/superset/pull/41285#discussion_r4081161972
##########
superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx:
##########
@@ -397,6 +400,93 @@ describe('SqlEditor', () => {
).toBeInTheDocument();
});
+ test('renders a registered northPane view in place of the editor', async ()
=> {
+ const { queryEditor } = mockedProps;
+ // The fixture has no tabViewId, so the component falls back to the id;
+ // mirror that here to derive the same persistence key.
+ const storageKey = `sqllab.northPaneView.${queryEditor.id}`;
+ localStorage.setItem(storageKey, 'test.northPane');
+ const disposable = views.registerView(
+ { id: 'test.northPane', name: 'Test North Pane' },
+ ViewLocations.sqllab.northPane,
+ () => <div data-test="np-view">NorthPane content</div>,
+ );
+
+ try {
+ const { findByTestId, queryByTestId } = setup(mockedProps, store);
+ expect(await findByTestId('np-view')).toBeInTheDocument();
+ // The default SQL editor pane is replaced, not rendered alongside.
+ expect(queryByTestId('react-ace')).not.toBeInTheDocument();
+ } finally {
+ disposable.dispose();
+ localStorage.removeItem(storageKey);
+ }
+ });
+
+ test('opens the northPane view carried on the query editor and persists the
per-tab key', async () => {
+ const { queryEditor } = mockedProps;
+ // The fixture has no tabViewId, so the component falls back to the id.
+ const storageKey = `sqllab.northPaneView.${queryEditor.id}`;
+ const disposable = views.registerView(
+ { id: 'test.northPane', name: 'Test North Pane' },
+ ViewLocations.sqllab.northPane,
+ () => <div data-test="np-view">NorthPane content</div>,
+ );
+
+ try {
+ // createTab({ northPaneViewId }) stamps the view onto the tab's own
+ // state, so nothing shared between tabs is involved.
+ const { findByTestId, queryByTestId } = setup(
+ {
+ ...mockedProps,
+ queryEditor: { ...queryEditor, northPaneViewId: 'test.northPane' },
+ },
+ store,
+ );
+ expect(await findByTestId('np-view')).toBeInTheDocument();
+ expect(queryByTestId('react-ace')).not.toBeInTheDocument();
+ // The chosen view is persisted under the per-tab key so it survives a
+ // reload that rehydrates the editor without the field.
+ expect(localStorage.getItem(storageKey)).toEqual('test.northPane');
+ } finally {
+ disposable.dispose();
+ localStorage.removeItem(storageKey);
+ }
+ });
+
+ test('does not open a northPane view requested for a different tab', () => {
+ const { queryEditor } = mockedProps;
+ const storageKey = `sqllab.northPaneView.${queryEditor.id}`;
+ const disposable = views.registerView(
+ { id: 'test.northPane', name: 'Test North Pane' },
+ ViewLocations.sqllab.northPane,
+ () => <div data-test="np-view">NorthPane content</div>,
+ );
+ // Another tab was created with the view; this one mounts as a plain tab
+ // (as happens when a reload restores several tabs at once).
+ const otherTabStore = createStore({
+ ...mockInitialState,
+ sqlLab: {
+ ...mockInitialState.sqlLab,
+ queryEditors: [
+ ...mockInitialState.sqlLab.queryEditors,
+ { ...extraQueryEditor1, northPaneViewId: 'test.northPane' },
+ ],
Review Comment:
Good catch, fixed the fixture to update the existing `extraQueryEditor1`
entry instead of duplicating the id.
##########
superset-frontend/src/SqlLab/components/SqlEditor/index.tsx:
##########
@@ -1048,6 +1127,29 @@ const SqlEditor: FC<Props> = ({
>
<Skeleton active />
</div>
+ ) : northPaneViewId &&
+ northPaneViews.some(v => v.id === northPaneViewId) ? (
Review Comment:
Good catch, falls back to `queryPane()` now while the view is still
registering instead of the empty state.
--
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]
