Skip to content

Interactive Window input box tweaks #10029

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

Merged
Merged
Show file tree
Hide file tree
Changes from all 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 news/2 Fixes/9282.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Clean up interative window styling and set focus to input box if clicking in the interactive window.
11 changes: 8 additions & 3 deletions src/datascience-ui/history-react/interactivePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export class InteractivePanel extends React.Component<IInteractivePanelProps> {
<section id="main-panel-variable" aria-label={getLocString('DataScience.collapseVariableExplorerLabel', 'Variables')}>
{this.renderVariablePanel(this.props.baseTheme)}
</section>
<main id="main-panel-content" onScroll={this.handleScroll}>
<main id="main-panel-content" onClick={this.contentPanelClick} onScroll={this.handleScroll}>
{this.renderContentPanel(this.props.baseTheme)}
</main>
<section id="main-panel-footer" aria-label={getLocString('DataScience.editSection', 'Input new cells here')}>
Expand All @@ -82,6 +82,11 @@ export class InteractivePanel extends React.Component<IInteractivePanelProps> {
);
}

// If click is not handled by something else, focus our input box
private contentPanelClick = (_event: React.MouseEvent<HTMLDivElement, MouseEvent>) => {
this.props.focusInput();
};

private renderToolbarPanel() {
const variableExplorerTooltip = this.props.variableState.visible
? getLocString('DataScience.collapseVariableExplorerTooltip', 'Hide variables active in jupyter kernel')
Expand Down Expand Up @@ -216,7 +221,7 @@ export class InteractivePanel extends React.Component<IInteractivePanelProps> {
monacoTheme={this.props.monacoTheme}
font={this.props.font}
settings={this.props.settings}
focusPending={this.props.activateCount}
focusPending={this.props.focusPending}
/>
</ErrorBoundary>
</div>
Expand Down Expand Up @@ -280,7 +285,7 @@ export class InteractivePanel extends React.Component<IInteractivePanelProps> {
monacoTheme={this.props.monacoTheme}
font={this.props.font}
settings={this.props.settings}
focusPending={this.props.activateCount}
focusPending={this.props.focusPending}
/>
</ErrorBoundary>
</div>
Expand Down
1 change: 1 addition & 0 deletions src/datascience-ui/history-react/redux/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {

// See https://react-redux.js.org/using-react-redux/connect-mapdispatch#defining-mapdispatchtoprops-as-an-object
export const actionCreators = {
focusInput: (): CommonAction<never | undefined> => ({ type: CommonActionType.FOCUS_INPUT }),
refreshVariables: (newExecutionCount?: number): CommonAction<IRefreshVariablesRequest> => ({ type: CommonActionType.REFRESH_VARIABLES, payload: { newExecutionCount } }),
restartKernel: (): CommonAction<never | undefined> => ({ type: CommonActionType.RESTART_KERNEL }),
interruptKernel: (): CommonAction<never | undefined> => ({ type: CommonActionType.INTERRUPT_KERNEL }),
Expand Down
1 change: 1 addition & 0 deletions src/datascience-ui/history-react/redux/mapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type InteractiveReducerFunc<T> = ReducerFunc<IMainState, CommonActionType, T>;
export type InteractiveReducerArg<T = never | undefined> = ReducerArg<IMainState, CommonActionType, T>;

export class IInteractiveActionMapping {
public [CommonActionType.FOCUS_INPUT]: InteractiveReducerFunc<never | undefined>;
public [CommonActionType.RESTART_KERNEL]: InteractiveReducerFunc<never | undefined>;
public [CommonActionType.SELECT_KERNEL]: InteractiveReducerFunc<never | undefined>;
public [CommonActionType.SELECT_SERVER]: InteractiveReducerFunc<never | undefined>;
Expand Down
1 change: 1 addition & 0 deletions src/datascience-ui/history-react/redux/reducers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const reducerMap: IInteractiveActionMapping = {
[CommonActionType.CLICK_CELL]: Effects.clickCell,
[CommonActionType.UNFOCUS_CELL]: Effects.unfocusCell,
[CommonActionType.UNMOUNT]: Creation.unmount,
[CommonActionType.FOCUS_INPUT]: CommonEffects.focusInput,

// Messages from the webview (some are ignored)
[IncomingMessageActions.STARTCELL]: Creation.startCell,
Expand Down
14 changes: 13 additions & 1 deletion src/datascience-ui/interactive-common/common.css
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ body, html {
}

.cell-wrapper:focus {
outline-width: 0px;
outline-color:var(--override-selection-background, var(--vscode-editor-selectionBackground));
}

Expand All @@ -98,6 +99,13 @@ body, html {
display: block;
}

.edit-panel-colorized .cell-wrapper:focus {
outline-width: 0px;
}
.edit-panel .cell-wrapper:focus {
outline-width: 0px;
}

.cell-outer {
display:grid;
grid-template-columns: auto 1fr;
Expand Down Expand Up @@ -252,12 +260,16 @@ body, html {
.code-watermark {
position: absolute;
top: 3px;
left: 30px;
left: 3px;
z-index: 500;
font-style: italic;
color: var(--override-watermark-color, var(--vscode-panelTitle-inactiveForeground));
}

.code-watermark:focus {
outline-width: 0;
}

.collapse-input-svg-rotate {
transform: rotate(45deg);
transform-origin: 0% 100%;
Expand Down
4 changes: 2 additions & 2 deletions src/datascience-ui/interactive-common/mainState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export type IMainState = {
testMode?: boolean;
codeTheme: string;
settings?: IDataScienceExtraSettings;
activateCount: number;
focusPending: number;
monacoReady: boolean;
loaded: boolean;
kernel: IServerState;
Expand Down Expand Up @@ -134,7 +134,7 @@ export function generateTestState(filePath: string = '', editable: boolean = fal
},
codeTheme: 'Foo',
settings: defaultSettings,
activateCount: 0,
focusPending: 0,
monacoReady: true,
loaded: false,
testMode: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,11 @@ export namespace CommonEffects {
}

export function activate<T>(arg: CommonReducerArg<T>): IMainState {
return {
...arg.prevState,
activateCount: arg.prevState.activateCount + 1
};
return focusPending(arg.prevState);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both do the same thing currently, but I was keeping them as they represent different actions.

}

export function focusInput<T>(arg: CommonReducerArg<T>): IMainState {
return focusPending(arg.prevState);
}

export function handleLocInit<T>(arg: CommonReducerArg<T, string>): IMainState {
Expand Down Expand Up @@ -105,4 +106,11 @@ export namespace CommonEffects {
monacoTheme: Identifiers.GeneratedThemeName
};
}

function focusPending(prevState: IMainState): IMainState {
return {
...prevState,
focusPending: prevState.focusPending + 1
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export enum CommonActionType {
EXPAND_ALL = 'action.expand_all',
EXPORT = 'action.export',
FOCUS_CELL = 'action.focus_cell',
FOCUS_INPUT = 'action.focus_input',
GATHER_CELL = 'action.gather_cell',
GET_VARIABLE_DATA = 'action.get_variable_data',
GOTO_CELL = 'action.goto_cell',
Expand Down
2 changes: 1 addition & 1 deletion src/datascience-ui/interactive-common/redux/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function generateDefaultState(skipDefault: boolean, testMode: boolean, baseTheme
family: "Consolas, 'Courier New', monospace"
},
codeTheme: Identifiers.GeneratedThemeName,
activateCount: 0,
focusPending: 0,
monacoReady: testMode, // When testing, monaco starts out ready
loaded: false,
kernel: {
Expand Down
2 changes: 1 addition & 1 deletion src/datascience-ui/native-editor/nativeEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ export class NativeEditor extends React.Component<INativeEditorProps> {
editorOptions={this.props.editorOptions}
enableGather={this.props.settings.enableGather}
themeMatplotlibPlots={this.props.settings.themeMatplotlibPlots}
focusPending={this.props.activateCount}
focusPending={this.props.focusPending}
/>
</ErrorBoundary>
{lastLine}
Expand Down
44 changes: 44 additions & 0 deletions src/test/datascience/interactiveWindow.functional.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,50 @@ suite('DataScience Interactive Window output tests', () => {
}
);

runMountedTest(
'Click outside cells sets focus to input box',
async wrapper => {
// Create an interactive window so that it listens to the results.
const interactiveWindow = await getOrCreateInteractiveWindow(ioc);
await interactiveWindow.show();

// Type in the input box
const editor = getInteractiveEditor(wrapper);
typeCode(editor, 'a=1\na');

// Give focus to a random div
const reactDiv = wrapper
.find('div')
.first()
.getDOMNode();

const domDiv = reactDiv.querySelector('div');

if (domDiv && ioc.postMessage) {
domDiv.tabIndex = -1;
domDiv.focus();

// Click in content-panel-div, since this doesn't click on any valid click handlers this
// should set input back to the input box
wrapper
.find('div#content-panel-div')
.first()
.simulate('click');

// Then enter press shift + enter on the active element
const activeElement = document.activeElement;
if (activeElement) {
await submitInput(ioc, activeElement as HTMLTextAreaElement);
}
}

verifyHtmlOnCell(wrapper, 'InteractiveCell', '<span>1</span>', CellPosition.Last);
},
() => {
return ioc;
}
);

runMountedTest(
'Collapse / expand cell',
async wrapper => {
Expand Down