Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add global shortcut to export logs #2336

Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"lint:packages": "eslint \"packages/*/src/**/*.{ts,tsx,js,jsx}\"",
"preview": "lerna run --scope=@deephaven/{code-studio,embed-widget} preview --stream",
"preview:app": "lerna run --scope=@deephaven/code-studio preview --stream",
"preview:embed-widget": "lerna run --scope=@deephaven/embed-widget preview --stream",
"prestart": "npm run build:necessary",
"start": "run-p watch:types start:*",
"start:app": "lerna run start --scope=@deephaven/code-studio --stream",
Expand Down
15 changes: 14 additions & 1 deletion packages/app-utils/src/components/AppBootstrap.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import '@deephaven/components/scss/BaseStyleSheet.scss';
import { ClientBootstrap } from '@deephaven/jsapi-bootstrap';
import { useBroadcastLoginListener } from '@deephaven/jsapi-components';
import { type Plugin } from '@deephaven/plugin';
import { ContextActions, ContextMenuRoot } from '@deephaven/components';
import FontBootstrap from './FontBootstrap';
import PluginsBootstrap from './PluginsBootstrap';
import AuthBootstrap from './AuthBootstrap';
import ConnectionBootstrap from './ConnectionBootstrap';
import { getConnectOptions } from '../utils';
import { getConnectOptions, createExportLogsContextAction } from '../utils';
import FontsLoaded from './FontsLoaded';
import UserBootstrap from './UserBootstrap';
import ServerConfigBootstrap from './ServerConfigBootstrap';
Expand All @@ -19,6 +20,9 @@ export type AppBootstrapProps = {
/** URL of the server. */
serverUrl: string;

/** Properties included in support logs. */
logMetadata?: Record<string, unknown>;

/** URL of the plugins to load. */
pluginsUrl: string;

Expand All @@ -43,6 +47,7 @@ export function AppBootstrap({
pluginsUrl,
getCorePlugins,
serverUrl,
logMetadata,
children,
}: AppBootstrapProps): JSX.Element {
const clientOptions = useMemo(() => getConnectOptions(), []);
Expand All @@ -56,6 +61,12 @@ export function AppBootstrap({
});
}, []);
useBroadcastLoginListener(onLogin, onLogout);

const contextActions = useMemo(
() => [createExportLogsContextAction(logMetadata, true)],
[logMetadata]
);

return (
<Provider store={store}>
<FontBootstrap fontClassNames={fontClassNames}>
Expand All @@ -78,10 +89,12 @@ export function AppBootstrap({
</UserBootstrap>
</ServerConfigBootstrap>
</AuthBootstrap>
<ContextActions actions={contextActions} />
</ClientBootstrap>
</ThemeBootstrap>
</PluginsBootstrap>
</FontBootstrap>
<ContextMenuRoot />
</Provider>
);
}
Expand Down
25 changes: 25 additions & 0 deletions packages/app-utils/src/utils/createExportLogsContextAction.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { type ContextAction, GLOBAL_SHORTCUTS } from '@deephaven/components';
import { exportLogs, logHistory } from '@deephaven/log';
import { store } from '@deephaven/redux';

export function createExportLogsContextAction(
metadata?: Record<string, unknown>,
isGlobal = false
): ContextAction {
return {
action: () => {
exportLogs(
logHistory,
{
...metadata,
userAgent: navigator.userAgent,
},
store.getState()
);
},
shortcut: GLOBAL_SHORTCUTS.EXPORT_LOGS,
isGlobal,
};
}

export default createExportLogsContextAction;
1 change: 1 addition & 0 deletions packages/app-utils/src/utils/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './ConnectUtils';
export * from './createExportLogsContextAction';
5 changes: 5 additions & 0 deletions packages/code-studio/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ const pluginsURL = new URL(
document.baseURI
);

const logMetadata: Record<string, unknown> = {
uiVersion: import.meta.env.npm_package_version,
};

// Lazy load the configs because it breaks initial page loads otherwise
async function getCorePlugins() {
const dashboardCorePlugins = await import(
Expand Down Expand Up @@ -69,6 +73,7 @@ ReactDOM.render(
getCorePlugins={getCorePlugins}
serverUrl={apiURL.origin}
pluginsUrl={pluginsURL.href}
logMetadata={logMetadata}
>
<AppRoot />
</AppBootstrap>
Expand Down
3 changes: 1 addition & 2 deletions packages/code-studio/src/main/App.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import React, { type ReactElement } from 'react';
import { ContextMenuRoot, ToastContainer } from '@deephaven/components';
import { ToastContainer } from '@deephaven/components';
ericlln marked this conversation as resolved.
Show resolved Hide resolved
import AppMainContainer from './AppMainContainer';

function App(): ReactElement {
return (
<div className="app">
<AppMainContainer />
<ContextMenuRoot />
<ToastContainer />
</div>
);
Expand Down
2 changes: 1 addition & 1 deletion packages/code-studio/src/main/AppMainContainer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function renderAppMainContainer({
setActiveTool = jest.fn(),
setDashboardIsolatedLinkerPanelId = jest.fn(),
client = new (dh as any).Client({}),
serverConfigValues = {},
serverConfigValues = new Map<string, string>(),
dashboardOpenedPanelMaps = {},
connection = makeConnection(),
session = makeSession(),
Expand Down
18 changes: 15 additions & 3 deletions packages/code-studio/src/main/AppMainContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,18 @@ import {
AppDashboards,
type LayoutStorage,
UserLayoutUtils,
createExportLogsContextAction,
} from '@deephaven/app-utils';
import JSZip from 'jszip';
import SettingsMenu from '../settings/SettingsMenu';
import AppControlsMenu from './AppControlsMenu';
import { getLayoutStorage, getServerConfigValues } from '../redux';
import './AppMainContainer.scss';
import WidgetList, { type WindowMouseEvent } from './WidgetList';
import { getFormattedVersionInfo } from '../settings/SettingsUtils';
import {
getFormattedPluginInfo,
getFormattedVersionInfo,
} from '../settings/SettingsUtils';
import EmptyDashboard from './EmptyDashboard';

const log = Log.module('AppMainContainer');
Expand Down Expand Up @@ -186,18 +190,26 @@ export class AppMainContainer extends Component<

this.importElement = React.createRef();

const { allDashboardData } = this.props;
const { allDashboardData, serverConfigValues, plugins } = this.props;

this.dashboardLayouts = new Map();
this.createDashboardListenerRemovers = new Map();
this.closeDashboardListenerRemovers = new Map();

this.state = {
contextActions: [
createExportLogsContextAction(
{
uiVersion: import.meta.env.npm_package_version,
userAgent: navigator.userAgent,
...Object.fromEntries(serverConfigValues),
pluginInfo: getFormattedPluginInfo(plugins),
},
false // Not global to prevent conflict with export logs action with same shortcut in AppBootstrap.tsx
),
{
action: () => {
// Copies the version info to the clipboard for easy pasting into a ticket
const { serverConfigValues } = this.props;
const versionInfo = getFormattedVersionInfo(serverConfigValues);
const versionInfoText = Object.entries(versionInfo)
.map(([key, value]) => `${key}: ${value}`)
Expand Down
7 changes: 7 additions & 0 deletions packages/components/src/shortcuts/GlobalShortcuts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ const GLOBAL_SHORTCUTS = {
macShortcut: [MODIFIER.CMD, MODIFIER.SHIFT, KEY.I],
isEditable: true,
}),
EXPORT_LOGS: ShortcutRegistry.createAndAdd({
id: 'GLOBAL.EXPORT_LOGS',
name: 'Export Logs',
shortcut: [MODIFIER.CTRL, MODIFIER.ALT, MODIFIER.SHIFT, KEY.L],
macShortcut: [MODIFIER.CMD, MODIFIER.OPTION, MODIFIER.SHIFT, KEY.L],
isEditable: true,
}),
NEXT: ShortcutRegistry.createAndAdd({
id: 'GLOBAL.NEXT',
name: 'Next',
Expand Down
2 changes: 0 additions & 2 deletions packages/embed-widget/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
import type GoldenLayout from '@deephaven/golden-layout';
import type { ItemConfig } from '@deephaven/golden-layout';
import {
ContextMenuRoot,
ErrorBoundary,
LoadingOverlay,
Shortcut,
Expand Down Expand Up @@ -241,7 +240,6 @@ function App(): JSX.Element {
errorMessage={error ?? null}
/>
)}
<ContextMenuRoot />
<ToastContainer />
</div>
);
Expand Down
4 changes: 4 additions & 0 deletions packages/embed-widget/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ const pluginsURL = new URL(
document.baseURI
);

const logMetadata: Record<string, unknown> = {
uiVersion: import.meta.env.npm_package_version,
};
// Lazy load the configs because it breaks initial page loads otherwise
async function getCorePlugins() {
const dashboardCorePlugins = await import(
Expand All @@ -59,6 +62,7 @@ ReactDOM.render(
getCorePlugins={getCorePlugins}
serverUrl={apiURL.origin}
pluginsUrl={pluginsURL.href}
logMetadata={logMetadata}
>
<App />
</AppBootstrap>
Expand Down
2 changes: 1 addition & 1 deletion packages/embed-widget/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default defineConfig(({ mode }) => {

let port = Number.parseInt(env.PORT, 10);
if (Number.isNaN(port) || port <= 0) {
port = 4030;
port = 4010;
ericlln marked this conversation as resolved.
Show resolved Hide resolved
}

const baseURL = new URL(env.BASE_URL, `http://localhost:${port}/`);
Expand Down
24 changes: 16 additions & 8 deletions playwright-ci.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,22 @@ import DefaultConfig from './playwright.config';

const config: PlaywrightTestConfig = {
...DefaultConfig,
webServer: {
// Only start the main code-studio server right now
// To test embed-widget, should have an array set for `webServer` and run them all separately as there's a port check
command: 'BASE_URL=/ide/ npm run preview:app -- -- -- --no-open', // Passing flags through npm is fun
port: 4000,
timeout: 60 * 1000,
reuseExistingServer: false,
},
webServer: [
{
command: 'BASE_URL=/ide/ npm run preview:app -- -- -- --no-open', // Passing flags through npm is fun
port: 4000,
timeout: 60 * 1000,
reuseExistingServer: false,
},
{
command:
'BASE_URL=/iframe/widget/ npm run preview:embed-widget -- -- -- --no-open',
port: 4010,
timeout: 60 * 1000,
reuseExistingServer: false,
},
],

// Applies to the npm command and CI, but CI will get overwritten in the CI config
reporter: [['github'], ['html']],
};
Expand Down
43 changes: 43 additions & 0 deletions tests/shortcuts.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { test, expect } from '@playwright/test';
import { gotoPage } from './utils';

test('shortcut downloads logs', async ({ page }) => {
await gotoPage(page, '');

const downloadPromise = page.waitForEvent('download');
await page.keyboard.press('ControlOrMeta+Alt+Shift+KeyL');
const download = await downloadPromise;

expect(download).not.toBeNull();
});

test('shortcut downloads logs in full screen error', async ({ page }) => {
// Go to embed-widget page without url parameter to trigger a full screen error
await gotoPage(page, 'http://localhost:4010/');

const downloadPromise = page.waitForEvent('download');
await page.keyboard.press('ControlOrMeta+Alt+Shift+KeyL');
const download = await downloadPromise;

expect(download).not.toBeNull();
});

test('shortcut downloads logs in embeded-widget', async ({ page }) => {
test.slow(); // Extend timeout to prevent a failure before page loads
ericlln marked this conversation as resolved.
Show resolved Hide resolved

// The embed-widgets page and the table itself have seperate loading spinners,
ericlln marked this conversation as resolved.
Show resolved Hide resolved
// causing a strict mode violation intermittently when using the goToPage helper
await gotoPage(page, 'http://localhost:4010?name=all_types');
await expect(
page.getByRole('progressbar', {
name: 'Loading...',
exact: true,
})
).toHaveCount(0);

const downloadPromise = page.waitForEvent('download');
await page.keyboard.press('ControlOrMeta+Alt+Shift+KeyL');
const download = await downloadPromise;

expect(download).not.toBeNull();
});
Loading