Skip to content

Commit fa857f0

Browse files
author
David Kutugata
authored
row height on data viewer scales to font size (#6875)
* row height on data viewer now scales to the font size of VS code * refactored constructor in the data viewer mainPanel.tsx * requested changes * added detail to a comment
1 parent 91a97e1 commit fa857f0

7 files changed

Lines changed: 73 additions & 50 deletions

File tree

‎news/2 Fixes/6614.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Make data viewer change row height according to font size in settings.

‎src/client/common/application/webPanel.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export class WebPanel implements IWebPanel {
3636
this.panel = window.createWebviewPanel(
3737
title.toLowerCase().replace(' ', ''),
3838
title,
39-
{viewColumn , preserveFocus: true},
39+
{ viewColumn, preserveFocus: true },
4040
{
4141
enableScripts: true,
4242
retainContextWhenHidden: true,
@@ -58,11 +58,11 @@ export class WebPanel implements IWebPanel {
5858
}
5959
}
6060

61-
public isVisible() : boolean {
61+
public isVisible(): boolean {
6262
return this.panel ? this.panel.visible : false;
6363
}
6464

65-
public isActive() : boolean {
65+
public isActive(): boolean {
6666
return this.panel ? this.panel.active : false;
6767
}
6868

@@ -121,7 +121,7 @@ export class WebPanel implements IWebPanel {
121121
private generateReactHtml(mainScriptPath: string, embeddedCss?: string, settings?: any) {
122122
const uriBasePath = Uri.file(`${path.dirname(mainScriptPath)}/`);
123123
const uriPath = Uri.file(mainScriptPath);
124-
const uriBase = uriBasePath.with({ scheme: 'vscode-resource'});
124+
const uriBase = uriBasePath.with({ scheme: 'vscode-resource' });
125125
const uri = uriPath.with({ scheme: 'vscode-resource' });
126126
const locDatabase = localize.getCollectionJSON();
127127
const style = embeddedCss ? embeddedCss : '';

‎src/client/datascience/codeCssGenerator.ts‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ const ThreeColorRegEx = /^#?([0-9A-Fa-f])([0-9A-Fa-f])([0-9A-Fa-f])$/;
2323
// These are based on the colors generated by 'Default Light+' and are only set when we
2424
// are ignoring themes.
2525
//tslint:disable:no-multiline-string object-literal-key-quotes
26-
const DefaultCssVars: { [key: string] : string } = {
27-
'light' : `
26+
const DefaultCssVars: { [key: string]: string } = {
27+
'light': `
2828
:root {
2929
--override-widget-background: #f3f3f3;
3030
--override-foreground: #000000;
@@ -38,7 +38,7 @@ const DefaultCssVars: { [key: string] : string } = {
3838
--override-peek-background: #f2f8fc;
3939
}
4040
`,
41-
'dark' : `
41+
'dark': `
4242
:root {
4343
--override-widget-background: #1e1e1e;
4444
--override-foreground: #d4d4d4;
@@ -56,25 +56,25 @@ const DefaultCssVars: { [key: string] : string } = {
5656

5757
// These colors below should match colors that come from either the Default Light+ theme or the Default Dark+ theme.
5858
// They are used when we can't find a theme json file.
59-
const DefaultColors: { [key: string] : string } = {
60-
'light.comment' : '#008000',
59+
const DefaultColors: { [key: string]: string } = {
60+
'light.comment': '#008000',
6161
'light.constant.numeric': '#09885a',
62-
'light.string' : '#a31515',
63-
'light.keyword.control' : '#AF00DB',
62+
'light.string': '#a31515',
63+
'light.keyword.control': '#AF00DB',
6464
'light.keyword.operator': '#000000',
65-
'light.variable' : '#001080',
65+
'light.variable': '#001080',
6666
'light.entity.name.type': '#267f99',
6767
'light.support.function': '#795E26',
68-
'light.punctuation' : '#000000',
69-
'dark.comment' : '#6A9955',
70-
'dark.constant.numeric' : '#b5cea8',
71-
'dark.string' : '#ce9178',
72-
'dark.keyword.control' : '#C586C0',
73-
'dark.keyword.operator' : '#d4d4d4',
74-
'dark.variable' : '#9CDCFE',
75-
'dark.entity.name.type' : '#4EC9B0',
76-
'dark.support.function' : '#DCDCAA',
77-
'dark.punctuation' : '#1e1e1e'
68+
'light.punctuation': '#000000',
69+
'dark.comment': '#6A9955',
70+
'dark.constant.numeric': '#b5cea8',
71+
'dark.string': '#ce9178',
72+
'dark.keyword.control': '#C586C0',
73+
'dark.keyword.operator': '#d4d4d4',
74+
'dark.variable': '#9CDCFE',
75+
'dark.entity.name.type': '#4EC9B0',
76+
'dark.support.function': '#DCDCAA',
77+
'dark.punctuation': '#1e1e1e'
7878
};
7979

8080
interface IApplyThemeArgs {
@@ -105,11 +105,11 @@ export class CodeCssGenerator implements ICodeCssGenerator {
105105
return this.applyThemeData(isDark, theme, '', this.generateCss.bind(this));
106106
}
107107

108-
public generateMonacoTheme(isDark: boolean, theme: string) : Promise<JSONObject> {
108+
public generateMonacoTheme(isDark: boolean, theme: string): Promise<JSONObject> {
109109
return this.applyThemeData(isDark, theme, {}, this.generateMonacoThemeObject.bind(this));
110110
}
111111

112-
private async applyThemeData<T>(isDark: boolean, theme: string, defaultT: T, applier: (args: IApplyThemeArgs) => T) : Promise<T> {
112+
private async applyThemeData<T>(isDark: boolean, theme: string, defaultT: T, applier: (args: IApplyThemeArgs) => T): Promise<T> {
113113
let result = defaultT;
114114
try {
115115
// First compute our current theme.
@@ -132,8 +132,8 @@ export class CodeCssGenerator implements ICodeCssGenerator {
132132
result = applier({ tokenColors, baseColors, fontFamily, fontSize, isDark: isDarkUpdated, defaultStyle: ignoreTheme ? LightTheme : undefined });
133133
} else if (tokenColors === null && fontFamily && fontSize) {
134134
// No colors found. See if we can figure out what type of theme we have
135-
const style = isDark ? DarkTheme : LightTheme ;
136-
result = applier({ fontFamily, fontSize, isDark: isDarkUpdated, defaultStyle: style});
135+
const style = isDark ? DarkTheme : LightTheme;
136+
result = applier({ fontFamily, fontSize, isDark: isDarkUpdated, defaultStyle: style });
137137
}
138138
}
139139
} catch (err) {
@@ -144,14 +144,14 @@ export class CodeCssGenerator implements ICodeCssGenerator {
144144
return result;
145145
}
146146

147-
private getScopes(entry: any) : JSONArray {
147+
private getScopes(entry: any): JSONArray {
148148
if (entry && entry.scope) {
149149
return Array.isArray(entry.scope) ? entry.scope as JSONArray : entry.scope.toString().split(',');
150150
}
151151
return [];
152152
}
153153

154-
private matchTokenColor(tokenColors: JSONArray, scope: string) : number {
154+
private matchTokenColor(tokenColors: JSONArray, scope: string): number {
155155
return tokenColors.findIndex((entry: any) => {
156156
const scopeArray = this.getScopes(entry);
157157
if (scopeArray.find(v => v !== null && v !== undefined && v.toString().trim() === scope)) {
@@ -184,7 +184,7 @@ export class CodeCssGenerator implements ICodeCssGenerator {
184184
return { color: this.getDefaultColor(defaultStyle, scope), fontStyle: 'normal' };
185185
}
186186

187-
private getDefaultColor(style: string | undefined, scope: string) : string {
187+
private getDefaultColor(style: string | undefined, scope: string): string {
188188
return style ? DefaultColors[`${style}.${scope}`] : 'var(--override-foreground, var(--vscode-editor-foreground))';
189189
}
190190

@@ -210,14 +210,14 @@ export class CodeCssGenerator implements ICodeCssGenerator {
210210
--code-font-size: ${args.fontSize}px;
211211
}
212212
213-
${args.defaultStyle ? DefaultCssVars[args.defaultStyle] : undefined }
213+
${args.defaultStyle ? DefaultCssVars[args.defaultStyle] : undefined}
214214
`;
215215
}
216216

217217
// Based on this data here:
218218
// https://github.com/Microsoft/vscode/blob/master/src/vs/editor/standalone/common/themes.ts#L13
219219
// tslint:disable: max-func-body-length
220-
private generateMonacoThemeObject(args: IApplyThemeArgs) : monacoEditor.editor.IStandaloneThemeData {
220+
private generateMonacoThemeObject(args: IApplyThemeArgs): monacoEditor.editor.IStandaloneThemeData {
221221
const result: monacoEditor.editor.IStandaloneThemeData = {
222222
base: args.isDark ? 'vs-dark' : 'vs',
223223
inherit: false,
@@ -295,7 +295,7 @@ export class CodeCssGenerator implements ICodeCssGenerator {
295295
});
296296
} else {
297297
// Otherwise use our default values.
298-
result.base = args.defaultStyle === DarkTheme ? 'vs-dark' : 'vs';
298+
result.base = args.defaultStyle === DarkTheme ? 'vs-dark' : 'vs';
299299
result.inherit = true;
300300

301301
if (args.defaultStyle) {
@@ -325,8 +325,8 @@ export class CodeCssGenerator implements ICodeCssGenerator {
325325
return [...colors1, ...colors2];
326326
}
327327

328-
private mergeBaseColors = (colors1: JSONObject, colors2: JSONObject) : JSONObject => {
329-
return {...colors1, ...colors2};
328+
private mergeBaseColors = (colors1: JSONObject, colors2: JSONObject): JSONObject => {
329+
return { ...colors1, ...colors2 };
330330
}
331331

332332
private readTokenColors = async (themeFile: string): Promise<JSONArray> => {

‎src/client/datascience/data-viewing/dataViewer.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { DataViewerMessages, IDataViewerMapping, IGetRowsRequest } from './types
2525
@injectable()
2626
export class DataViewer extends WebViewHost<IDataViewerMapping> implements IDataViewer, IDisposable {
2727
private disposed: boolean = false;
28-
private variable : IJupyterVariable | undefined;
28+
private variable: IJupyterVariable | undefined;
2929
private rowsTimer: StopWatch | undefined;
3030
private pendingRowsCount: number = 0;
3131

@@ -37,7 +37,7 @@ export class DataViewer extends WebViewHost<IDataViewerMapping> implements IData
3737
@inject(IWorkspaceService) workspaceService: IWorkspaceService,
3838
@inject(IJupyterVariables) private variableManager: IJupyterVariables,
3939
@inject(IApplicationShell) private applicationShell: IApplicationShell
40-
) {
40+
) {
4141
super(
4242
configuration,
4343
provider,
@@ -90,13 +90,13 @@ export class DataViewer extends WebViewHost<IDataViewerMapping> implements IData
9090
super.onMessage(message, payload);
9191
}
9292

93-
private async prepVariable(variable: IJupyterVariable) : Promise<IJupyterVariable> {
93+
private async prepVariable(variable: IJupyterVariable): Promise<IJupyterVariable> {
9494
this.rowsTimer = new StopWatch();
9595
const output = await this.variableManager.getDataFrameInfo(variable);
9696

9797
// Log telemetry about number of rows
9898
try {
99-
sendTelemetryEvent(Telemetry.ShowDataViewer, 0, {rows: output.rowCount ? output.rowCount : 0, columns: output.columns ? output.columns.length : 0 });
99+
sendTelemetryEvent(Telemetry.ShowDataViewer, 0, { rows: output.rowCount ? output.rowCount : 0, columns: output.columns ? output.columns.length : 0 });
100100

101101
// Count number of rows to fetch so can send telemetry on how long it took.
102102
this.pendingRowsCount = output.rowCount ? output.rowCount : 0;

‎src/datascience-ui/data-explorer/mainPanel.tsx‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ interface IMainPanelState {
4040
totalRowCount: number;
4141
filters: {};
4242
indexColumn: string;
43+
styleReady: boolean;
4344
}
4445

4546
export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState> implements IMessageHandler {
@@ -65,21 +66,21 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
6566
totalRowCount: data.rows.length,
6667
fetchedRowCount: 0,
6768
filters: {},
68-
indexColumn: data.primaryKeys[0]
69+
indexColumn: data.primaryKeys[0],
70+
styleReady: false
6971
};
7072

7173
// Fire off a timer to mimic dynamic loading
72-
setTimeout(() => {
73-
this.handleGetAllRowsResponse({data: data.rows});
74-
}, 1000);
74+
setTimeout(() => this.handleGetAllRowsResponse({data: data.rows}), 1000);
7575
} else {
7676
this.state = {
7777
gridColumns: [],
7878
gridRows: [],
7979
totalRowCount: 0,
8080
fetchedRowCount: 0,
8181
filters: {},
82-
indexColumn: 'index'
82+
indexColumn: 'index',
83+
styleReady: false
8384
};
8485
}
8586
}
@@ -110,10 +111,11 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
110111
return (
111112
<div className='main-panel' ref={this.container}>
112113
<StyleInjector
114+
onReady={this.saveReadyState}
113115
expectingDark={this.props.baseTheme !== 'vscode-light'}
114116
postOffice={this.postOffice} />
115117
{progressBar}
116-
{this.state.totalRowCount > 0 && this.renderGrid()}
118+
{this.state.totalRowCount > 0 && this.state.styleReady && this.renderGrid()}
117119
</div>
118120
);
119121
}
@@ -140,6 +142,10 @@ export class MainPanel extends React.Component<IMainPanelProps, IMainPanelState>
140142
return false;
141143
}
142144

145+
private saveReadyState = () => {
146+
this.setState({styleReady: true});
147+
}
148+
143149
private renderGrid() {
144150
const filterRowsText = getLocString('DataScience.filterRowsButton', 'Filter Rows');
145151
const filterRowsTooltip = getLocString('DataScience.filterRowsTooltip', 'Click to filter.');

‎src/datascience-ui/data-explorer/reactSlickGrid.tsx‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import './reactSlickGrid.css';
3434

3535
const MinColumnWidth = 70;
3636
const MaxColumnWidth = 500;
37-
const RowHeightAdjustment = 4;
3837

3938
export interface ISlickRow extends Slick.SlickData {
4039
id: string;
@@ -147,21 +146,21 @@ export class ReactSlickGrid extends React.Component<ISlickGridProps, ISlickGridS
147146

148147
if (this.containerRef.current) {
149148
// Compute font size. Default to 15 if not found.
150-
let fontSize = parseInt(getComputedStyle(this.containerRef.current).getPropertyValue('--vscode-font-size'), 10);
149+
let fontSize = parseInt(getComputedStyle(this.containerRef.current).getPropertyValue('--code-font-size'), 10);
151150
if (isNaN(fontSize)) {
152151
fontSize = 15;
153152
}
154153

155154
// Setup options for the grid
156-
const options : Slick.GridOptions<Slick.SlickData> = {
155+
const options: Slick.GridOptions<Slick.SlickData> = {
157156
asyncEditorLoading: true,
158157
editable: false,
159158
enableCellNavigation: true,
160159
showHeaderRow: true,
161160
enableColumnReorder: false,
162161
explicitInitialization: false,
163162
viewportClass: 'react-grid',
164-
rowHeight: fontSize + RowHeightAdjustment
163+
rowHeight: this.getAppropiateRowHeight(fontSize)
165164
};
166165

167166
// Transform columns so they are sortable and stylable
@@ -296,6 +295,22 @@ export class ReactSlickGrid extends React.Component<ISlickGridProps, ISlickGridS
296295
args.grid.render();
297296
}
298297

298+
// These adjustments for the row height come from trial and error, by changing the font size in VS code,
299+
// opening a new Data Viewer, and making sure the data is visible
300+
// They were tested up to a font size of 60, and the row height still allows the content to be seen
301+
private getAppropiateRowHeight(fontSize: number): number {
302+
switch (true) {
303+
case (fontSize < 15):
304+
return fontSize + 4;
305+
case (fontSize < 20):
306+
return fontSize + 8;
307+
case (fontSize < 30):
308+
return fontSize + 10;
309+
default:
310+
return fontSize + 12;
311+
}
312+
}
313+
299314
// If the slickgrid gets focus and nothing is selected select the first item
300315
// so that you can keyboard navigate from there
301316
private slickgridFocus = (_e: any): void => {
@@ -366,7 +381,7 @@ export class ReactSlickGrid extends React.Component<ISlickGridProps, ISlickGridS
366381
const document = this.containerRef.current.ownerDocument;
367382
if (document) {
368383
const cssOverrideNode = document.createElement('style');
369-
const rule = `.${gridName} .slick-cell {height: ${this.state.fontSize + RowHeightAdjustment}px;}`;
384+
const rule = `.${gridName} .slick-cell {height: ${this.getAppropiateRowHeight(this.state.fontSize)}px;}`;
370385
cssOverrideNode.setAttribute('type', 'text/css');
371386
cssOverrideNode.setAttribute('rel', 'stylesheet');
372387
cssOverrideNode.appendChild(document.createTextNode(rule));

‎src/datascience-ui/react-common/styleInjector.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export interface IStyleInjectorProps {
1616
postOffice: PostOffice;
1717
darkChanged?(newDark: boolean): void;
1818
monacoThemeChanged?(theme: string): void;
19+
onReady?(): void;
1920
}
2021

2122
interface IStyleInjectorState {
@@ -104,7 +105,7 @@ export class StyleInjector extends React.Component<IStyleInjectorProps, IStyleIn
104105
rootCss: response.css,
105106
theme: response.theme,
106107
knownDark: computedKnownDark
107-
});
108+
}, this.props.onReady);
108109
}
109110
}
110111

0 commit comments

Comments
 (0)