Skip to content

Commit

Permalink
Fix scrollbar thumb crash and UX
Browse files Browse the repository at this point in the history
  • Loading branch information
pradeepnschrodinger committed Mar 29, 2024
1 parent 24fb75a commit a674634
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 66 deletions.
34 changes: 14 additions & 20 deletions src/css/layout/ScrollbarLayout.css
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,18 @@
.ScrollbarLayout/mainHorizontal {
height: var(--scrollbar-size);
left: 0;
transition-property: background-color height;
}

/* Touching the scroll-track directly makes the scroll-track bolder */
.ScrollbarLayout/mainHorizontal.public/Scrollbar/mainActive,
.ScrollbarLayout/mainHorizontal:hover {
height: var(--scrollbar-size-large);
}

.ScrollbarLayout/face {
left: 0;
overflow: hidden;
position: absolute;
z-index: 1;
transition-duration: 250ms;
transition-timing-function: ease;
transition-property: width;

/* keep the thumb aligned to the center */
display: flex;
justify-content: center;
align-items: center;
}

/**
Expand All @@ -57,7 +52,9 @@
content: '';
display: block;
position: absolute;
transition: background-color 250ms ease;
transition-duration: 250ms;
transition-timing-function: ease;
transition-property: background-color, height, width;
}

.ScrollbarLayout/faceHorizontal {
Expand All @@ -67,10 +64,8 @@
}

.ScrollbarLayout/faceHorizontal:after {
bottom: var(--scrollbar-face-margin);
left: 0;
top: var(--scrollbar-face-margin);
width: 100%;
height: calc(100% - var(--scrollbar-face-margin) * 2);
}

.fixedDataTable_isRTL .ScrollbarLayout/faceHorizontal,
Expand All @@ -79,9 +74,10 @@
left: auto;
}

/* expand horizontal scrollbar face when active */
.ScrollbarLayout/faceHorizontal.public/Scrollbar/faceActive:after,
.ScrollbarLayout/main:hover .ScrollbarLayout/faceHorizontal:after {
bottom: calc(var(--scrollbar-face-margin)/2);
height: calc(100% - var(--scrollbar-face-margin));
}

.ScrollbarLayout/faceVertical {
Expand All @@ -92,13 +88,11 @@

.ScrollbarLayout/faceVertical:after {
height: 100%;
left: var(--scrollbar-face-margin);
right: var(--scrollbar-face-margin);
top: 0;
width: calc(100% - var(--scrollbar-face-margin) * 2);
}

/* expand veritcal scrollbar face when active */
.ScrollbarLayout/main:hover .ScrollbarLayout/faceVertical:after,
.ScrollbarLayout/faceVertical.public/Scrollbar/faceActive:after {
left: calc(var(--scrollbar-face-margin)/2);
right: calc(var(--scrollbar-face-margin)/2);
width: calc(100% - var(--scrollbar-face-margin));
}
48 changes: 3 additions & 45 deletions src/plugins/Scrollbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ const FACE_MARGIN_2 = FACE_MARGIN * 2;
const FACE_SIZE_MIN = 30;
const KEYBOARD_SCROLL_AMOUNT = 40;

let _lastScrolledScrollbar = null;

class Scrollbar extends React.PureComponent {
static propTypes = {
contentSize: PropTypes.number.isRequired,
Expand Down Expand Up @@ -110,7 +108,7 @@ class Scrollbar extends React.PureComponent {
let faceStyle;
const isHorizontal = this.state.isHorizontal;
const isVertical = !isHorizontal;
const isActive = this.state.focused || this.state.isDragging;
const isActive = this.state.isDragging;
const faceSize = this.state.faceSize;
const isOpaque = this.props.isOpaque;
const verticalTop = this.props.verticalTop || 0;
Expand Down Expand Up @@ -182,8 +180,6 @@ class Scrollbar extends React.PureComponent {

return (
<div
onFocus={this._onFocus}
onBlur={this._onBlur}
onKeyDown={this._onKeyDown}
onMouseDown={this._onMouseDown}
onTouchCancel={this._onTouchCancel}
Expand Down Expand Up @@ -244,9 +240,6 @@ class Scrollbar extends React.PureComponent {
this._mouseMoveTracker.releaseMouseMoves();
this._mouseMoveTracker = null;
}
if (_lastScrolledScrollbar === this) {
_lastScrolledScrollbar = null;
}
}

scrollBy = (/*number*/ delta) => {
Expand Down Expand Up @@ -313,15 +306,10 @@ class Scrollbar extends React.PureComponent {
position = maxPosition;
}

const isDragging = this._mouseMoveTracker
? this._mouseMoveTracker.isDragging()
: false;

// This function should only return flat values that can be compared quiclky
// by `ReactComponentWithPureRenderMixin`.
const state = {
faceSize,
isDragging,
isHorizontal,
position,
scale,
Expand Down Expand Up @@ -358,6 +346,8 @@ class Scrollbar extends React.PureComponent {
};

_onMouseDown = (/*object*/ event) => {
this.setState({ isDragging: true });

/** @type {object} */
let nextState;

Expand Down Expand Up @@ -387,7 +377,6 @@ class Scrollbar extends React.PureComponent {
nextState = {};
}

nextState.focused = true;
this._setNextState(nextState);

this._mouseMoveTracker.captureMouseMoves(event);
Expand Down Expand Up @@ -535,32 +524,6 @@ class Scrollbar extends React.PureComponent {
);
};

_onFocus = () => {
this.setState({
focused: true,
});
};

_onBlur = () => {
this.setState({
focused: false,
});
};

_blur = () => {
const el = ReactDOM.findDOMNode(this);
if (!el) {
return;
}

try {
this._onBlur();
el.blur();
} catch (oops) {
// pass
}
};

getTouchX = (/*object*/ e) => {
return Math.round(
e.targetTouches[0].clientX - e.target.getBoundingClientRect().x
Expand Down Expand Up @@ -593,11 +556,6 @@ class Scrollbar extends React.PureComponent {
}
return;
}

if (willScroll && _lastScrolledScrollbar !== this) {
_lastScrolledScrollbar && _lastScrolledScrollbar._blur();
_lastScrolledScrollbar = this;
}
};

_didScroll = () => {
Expand Down
1 change: 0 additions & 1 deletion src/stubs/cssVar.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const CSS_VARS = {
'--scrollbar-face-margin': '4px',
'--scrollbar-face-radius': '6px',
'--scrollbar-size': '15px',
'--scrollbar-size-large': '17px',
'--scrollbar-track-color': '#fff',
'--border-color': '#d3d3d3',
'--fbui-white': '#fff',
Expand Down

0 comments on commit a674634

Please sign in to comment.