Skip to content

More scrollbar fixes and some CSS cleanup #10300

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.

Sign up for GitHub

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
IanMatthewHuff merged 6 commits into from
Feb 25, 2020
Merged

More scrollbar fixes and some CSS cleanup #10300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
96d2459
scrollbar wip
IanMatthewHuff Feb 22, 2020
c1591dd
ui removal of vw and fixing of grids
IanMatthewHuff Feb 24, 2020
6beed48
add inner scrollbar size
IanMatthewHuff Feb 24, 2020
e35815f
remove old comment
IanMatthewHuff Feb 24, 2020
2e09dbb
added horizontal scroll setting as well
IanMatthewHuff Feb 24, 2020
7e400ca
more padding per pm feedback between scrollbars
IanMatthewHuff Feb 25, 2020
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 src/client/datascience/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,7 @@ export interface IDataScienceExtraSettings extends IDataScienceSettings {
autoSurround: string;
autoIndent: boolean;
scrollBeyondLastLine: boolean;
horizontalScrollbarSize: number;
verticalScrollbarSize: number;
fontSize: number;
fontFamily: string;
Expand Down
4 changes: 3 additions & 1 deletion src/client/datascience/webViewHost.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,9 @@ export abstract class WebViewHost<IMapping> implements IDisposable {
autoIndent: this.getValue(editor, 'autoIndent', false),
fontLigatures: this.getValue(editor, 'fontLigatures', false),
scrollBeyondLastLine: this.getValue(editor, 'scrollBeyondLastLine', true),
// VS Code puts a value for this, but it's 10 (the explorer bar size) not 14 the editor size
// VS Code puts a value for this, but it's 10 (the explorer bar size) not 14 the editor size for vert
verticalScrollbarSize: this.getValue(editor, 'scrollbar.verticalScrollbarSize', 14),
horizontalScrollbarSize: this.getValue(editor, 'scrollbar.horizontalScrollbarSize', 10),
fontSize: this.getValue(editor, 'fontSize', 14),
fontFamily: this.getValue(editor, 'fontFamily', "Consolas, 'Courier New', monospace")
},
Expand Down Expand Up @@ -343,6 +344,7 @@ export abstract class WebViewHost<IMapping> implements IDisposable {
event.affectsConfiguration('editor.scrollBeyondLastLine') ||
event.affectsConfiguration('editor.fontLigatures') ||
event.affectsConfiguration('editor.scrollbar.verticalScrollbarSize') ||
event.affectsConfiguration('editor.scrollbar.horizontalScrollbarSize') ||
event.affectsConfiguration('files.autoSave') ||
event.affectsConfiguration('files.autoSaveDelay') ||
event.affectsConfiguration('python.dataScience.enableGather')
Expand Down
14 changes: 5 additions & 9 deletions src/datascience-ui/history-react/interactivePanel.less
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
/* Import common styles and then override them below */
@import "../interactive-common/common.css";

:root {
--messages-offset-px: 20px;
--output-offset-px: 45px;
}

.toolbar-menu-bar-child {
background: var(--override-background, var(--vscode-editor-background));
z-index: 10;
Expand All @@ -19,7 +14,7 @@
}

.messages-result-container {
width: calc(100vw - var(--messages-offset-px));
width: 100%;
Copy link
Member Author

@IanMatthewHuff IanMatthewHuff Feb 25, 2020

Choose a reason for hiding this comment

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

Not fully sure why the viewport - magic was being used, but it might have been to work around the grid blowout issues that I fixed, or some issue that doesn't apply any move, so I removed them.

Copy link

@rchiodo rchiodo Feb 25, 2020

Choose a reason for hiding this comment

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

Yeah that's likely why I did it. I couldn't get the damn thing to use 100% of the width. It was probably the extra div messing it up.

In reply to: 383589621 [](ancestors = 383589621)

}

.messages-result-container pre {
Expand All @@ -38,19 +33,20 @@
.cell-result-container {
margin: 0px;
display: grid;
grid-auto-columns: minmax(0, 1fr);
Copy link
Member Author

@IanMatthewHuff IanMatthewHuff Feb 25, 2020

Choose a reason for hiding this comment

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

A few cases of this were long content was blowing out our grid cells.
https://stackoverflow.com/questions/52861086/how-come-minmax0-1fr-works-for-long-elements-while-1fr-doesnt
We would have grids that were 200px wide, with a 400+px column inside them due to long content blowing them out.

}

.cell-outer {
display:grid;
grid-template-columns: auto 1fr 8px;
grid-template-columns: auto minmax(0, 1fr) 8px;
grid-column-gap: 3px;
width: 100%;
}


.cell-output {
margin: 0px;
width: calc(100vw - var(--output-offset-px));
width: 100%;
overflow-x: scroll;
background: transparent;
}
Expand All @@ -68,7 +64,7 @@ xmp {
}

.markdown-cell-output {
width: calc(100vw - var(--output-offset-px));
width: 100%;
overflow-x: scroll;
}

Expand Down
1 change: 1 addition & 0 deletions src/datascience-ui/history-react/interactivePanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ ${buildSettingsCss(this.props.settings)}`}</style>
testMode: this.props.testMode,
codeTheme: this.props.codeTheme,
submittedText: this.props.submittedText,
settings: this.props.settings,
skipNextScroll: this.props.skipNextScroll ? true : false,
editable: false,
renderCell: this.renderCell,
Expand Down
8 changes: 8 additions & 0 deletions src/datascience-ui/interactive-common/buildSettingsCss.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ export function buildSettingsCss(settings: IDataScienceExtraSettings | undefined
return settings
? `#main-panel-content::-webkit-scrollbar {
width: ${settings.extraSettings.editor.verticalScrollbarSize}px;
}

.cell-output::-webkit-scrollbar {
Copy link
Member Author

@IanMatthewHuff IanMatthewHuff Feb 25, 2020

Choose a reason for hiding this comment

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

Added in H support as well.

height: ${settings.extraSettings.editor.horizontalScrollbarSize}px;
}

.cell-output > *::-webkit-scrollbar {
width: ${settings.extraSettings.editor.verticalScrollbarSize}px;
}`
: '';
}
8 changes: 2 additions & 6 deletions src/datascience-ui/interactive-common/common.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ body, html {
/* Cell */
.cell-wrapper {
margin: 0px;
padding: 2px;
padding: 2px 5px 2px 2px;
display: block;
}

Expand Down Expand Up @@ -310,14 +310,10 @@ body, html {
}

#cell-table {
display: table;
display: block;
Copy link
Member Author

@IanMatthewHuff IanMatthewHuff Feb 25, 2020

Choose a reason for hiding this comment

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

We had an extra cell-table / cell-table div group that looked like a remnant from an earlier layout.

width: 100%;
}

#cell-table-body {
display: table-row-group;
}

.flash {
animation-name: flash-animation;
animation-duration: 1.0s;
Expand Down
8 changes: 4 additions & 4 deletions src/datascience-ui/interactive-common/contentPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
'use strict';
import * as React from 'react';

import { IDataScienceExtraSettings } from '../../client/datascience/types';
import { InputHistory } from './inputHistory';
import { ICellViewModel } from './mainState';

Expand All @@ -16,6 +17,7 @@ export interface IContentPanelProps {
cellVMs: ICellViewModel[];
history?: InputHistory;
testMode?: boolean;
settings?: IDataScienceExtraSettings;
codeTheme: string;
submittedText: boolean;
skipNextScroll: boolean;
Expand Down Expand Up @@ -53,10 +55,8 @@ export class ContentPanel extends React.Component<IContentPanelProps> {
const className = `${this.props.scrollBeyondLastLine ? 'content-panel-scrollBeyondLastLine' : ''}`;
return (
<div id="content-panel-div" ref={this.containerRef} className={className}>
<div id="cell-table">
<div id="cell-table-body" role="list">
{this.renderCells()}
</div>
<div id="cell-table" role="list">
{this.renderCells()}
</div>
<div ref={this.bottomRef} />
</div>
Expand Down
4 changes: 2 additions & 2 deletions src/datascience-ui/native-editor/nativeCell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ export class NativeCell extends React.Component<INativeCellProps> {
private renderInput = () => {
if (this.shouldRenderInput()) {
return (
<div>
<div className="cell-input-wrapper">
Copy link
Member Author

@IanMatthewHuff IanMatthewHuff Feb 25, 2020

Choose a reason for hiding this comment

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

The wrappers let us place the entire input unit into the containing grid correctly (this is the issue I was talking about on the whiteboard in particular)

{this.renderMiddleToolbar()}
<CellInput
cellVM={this.props.cellVM}
Expand Down Expand Up @@ -674,7 +674,7 @@ export class NativeCell extends React.Component<INativeCellProps> {
const toolbar = this.props.cellVM.cell.data.cell_type === 'markdown' ? this.renderMiddleToolbar() : null;
if (this.shouldRenderOutput()) {
return (
<div>
<div className="cell-output-wrapper">
{toolbar}
<CellOutput
cellVM={this.props.cellVM}
Expand Down
40 changes: 16 additions & 24 deletions src/datascience-ui/native-editor/nativeEditor.less
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
/* Import common styles and then override them below */
@import "../interactive-common/common.css";

:root {
--right-offset-px: 94px;
}

.toolbar-panel-button {
border-width: 1px;
border-style: solid;
Expand All @@ -18,12 +14,6 @@
cursor: hand;
}

.measure-editor-div {
width: 90vw;
visibility: hidden;
position: absolute;
}

.cell-wrapper {
margin: 2px 2px 0px 0px;
position: relative;
Expand All @@ -42,16 +32,16 @@
justify-self: right;
}

// make the width of the div be a bit less than the width of the window,
// the 85 pixels removed account for the space the padding and controls
// on the left take in the native Editor.
.cell-output-wrapper {
grid-row: 1;
grid-column: 3;
}

.cell-output {
margin-top: 5px;
background: transparent;
width: calc(100vw - var(--right-offset-px));
width: 100%;
overflow-x: scroll;
grid-row: 1;
grid-column: 3;
}

.cell-output-text {
Expand All @@ -66,18 +56,18 @@
}

.markdown-cell-output {
width: calc(100vw - var(--right-offset-px));
width: 100%;
overflow-x: scroll;
}

.cell-outer {
display:grid;
grid-template-columns:auto auto 1fr;
grid-template-columns:auto auto minmax(0, 1fr);
}

.cell-outer-editable {
display:grid;
grid-template-columns:auto auto 1fr;
grid-template-columns:auto auto minmax(0, 1fr);
margin-top: 0px;
}

Expand Down Expand Up @@ -126,11 +116,13 @@
top: 5px; /* Account for extra padding and border in native editor */
}

.cell-input {
margin: 2px 10px 0px 0px;
.cell-input-wrapper {
grid-column: 3;
grid-row: 1;
min-width: calc(100% - var(--right-offset-px));
}

.cell-input {
margin: 2px 10px 0px 0px;
}

.content-div {
Expand Down Expand Up @@ -374,7 +366,7 @@
.add-cell-line-divider {
margin-top: 8px;
margin-left: 2px;
width: calc(100vw - 40px);
width: calc(100% - 40px);
border-width: 0px;
border-top-color: var(--override-badge-background, var(--vscode-badge-background));
border-top-width: 1px;
Expand All @@ -401,7 +393,7 @@ collapse-bar controls-div [cell-input, cell-output, markdown-cell-
*/
.cell-row-container {
display: grid;
grid-template-columns: auto auto 1fr;
grid-template-columns: auto auto minmax(0, 1fr);
grid-template-rows: 1fr auto;
}

Expand Down
3 changes: 2 additions & 1 deletion src/datascience-ui/react-common/settingsReactSide.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ export function getDefaultSettings() {
autoIndent: false,
fontLigatures: false,
scrollBeyondLastLine: true,
// VS Code puts a value for this, but it's 10 (the explorer bar size) not 14 the editor size
// VS Code puts a value for this, but it's 10 (the explorer bar size) not 14 the editor size for vert
verticalScrollbarSize: 14,
horizontalScrollbarSize: 14,
fontSize: 14,
fontFamily: "Consolas, 'Courier New', monospace"
},
Expand Down