Skip to content

Commit 5447c6e

Browse files
Improve Slider Scrolling (#8321)
* fix that slider could be scrolled to NaN * only scroll slider on focus * remove console logs * WIP: prevent parent scrolling * improve onWheel method * fix resetting to default on log sliders * add event listener with useEffect * remove event listener * add changelog * improve changelog wording * WIP: address review * fix opacity default for segmentation layers * disable text select when slider is focused * improve if clause
1 parent 3b946f8 commit 5447c6e

File tree

5 files changed

+88
-41
lines changed

5 files changed

+88
-41
lines changed

CHANGELOG.unreleased.md

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
1818
- Measurement tools are now accessible when viewing datasets outside of an annotation. [#8334](https://github.com/scalableminds/webknossos/pull/8334)
1919

2020
### Changed
21+
- Improved the scrolling behaviour of sliders: Sliders must be focused to scroll them. Additionally, parent element scrolling is prevented when using the slider. [#8321](https://github.com/scalableminds/webknossos/pull/8321) [#8321](https://github.com/scalableminds/webknossos/pull/8321)
2122

2223
### Fixed
2324
- Fixed a bug that lead to trees being dropped when merging to trees together. [#8359](https://github.com/scalableminds/webknossos/pull/8359)

frontend/javascripts/components/slider.tsx

+66-30
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@ import { Slider as AntdSlider, type SliderSingleProps } from "antd";
22
import type { SliderRangeProps } from "antd/lib/slider";
33
import { clamp } from "libs/utils";
44
import _ from "lodash";
5-
import type { WheelEventHandler } from "react";
5+
import { useCallback, useEffect, useRef, useState } from "react";
66

77
const DEFAULT_WHEEL_FACTOR = 0.02;
88
const DEFAULT_STEP = 1;
99

1010
type SliderProps = (SliderSingleProps | SliderRangeProps) & {
1111
wheelFactor?: number;
1212
onWheelDisabled?: boolean;
13+
onResetToDefault?: () => void;
1314
};
1415

1516
const getDiffPerSliderStep = (
@@ -23,6 +24,8 @@ const getDiffPerSliderStep = (
2324
};
2425

2526
const getWheelStepFromEvent = (step: number, deltaY: number, wheelStep: number) => {
27+
const absDeltaY = Math.abs(deltaY);
28+
if (absDeltaY === 0 || step === 0) return 0;
2629
// Make sure that result is a multiple of step
2730
return step * Math.round((wheelStep * deltaY) / Math.abs(deltaY) / step);
2831
};
@@ -32,6 +35,7 @@ export function Slider(props: SliderProps) {
3235
min,
3336
max,
3437
onChange,
38+
onResetToDefault,
3539
value,
3640
range,
3741
defaultValue,
@@ -40,49 +44,81 @@ export function Slider(props: SliderProps) {
4044
step,
4145
disabled,
4246
} = props;
47+
const isFocused = useRef(false);
48+
const sliderRef = useRef<HTMLDivElement>(null);
49+
50+
const handleWheelEvent = useCallback(
51+
(event: { preventDefault: () => void; deltaY: number }) => {
52+
if (
53+
onWheelDisabled ||
54+
value == null ||
55+
min == null ||
56+
max == null ||
57+
!isFocused.current ||
58+
onChange == null
59+
)
60+
return;
61+
event.preventDefault();
62+
const diff = getWheelStepFromEvent(ensuredStep, event.deltaY, wheelStep);
63+
// differentiate between single value and range slider
64+
if (range === false || range == null) {
65+
const newValue = value - diff;
66+
const clampedNewValue = clamp(min, newValue, max);
67+
onChange(clampedNewValue);
68+
} else if (range === true || typeof range === "object") {
69+
const newLowerValue = Math.round(value[0] + diff);
70+
const newUpperValue = Math.round(value[1] - diff);
71+
const clampedNewLowerValue = clamp(min, newLowerValue, Math.min(newUpperValue, max));
72+
const clampedNewUpperValue = clamp(newLowerValue, newUpperValue, max);
73+
onChange([clampedNewLowerValue, clampedNewUpperValue]);
74+
}
75+
},
76+
[value, min, max, onChange, range, onWheelDisabled],
77+
);
78+
79+
// Reacts onWheel is passive by default, this means that it can't preventDefault.
80+
// Thus we need to add the event listener manually.
81+
// (See https://github.com/facebook/react/pull/19654)
82+
useEffect(() => {
83+
const sliderElement = sliderRef.current;
84+
if (sliderElement) {
85+
sliderElement.addEventListener("wheel", handleWheelEvent, { passive: false });
86+
return () => {
87+
sliderElement.removeEventListener("wheel", handleWheelEvent);
88+
};
89+
}
90+
}, [handleWheelEvent]);
91+
4392
if (min == null || max == null || onChange == null || value == null || disabled)
4493
return <AntdSlider {...props} />;
4594
const sliderRange = max - min;
4695
const ensuredStep = step || DEFAULT_STEP;
47-
let handleWheelEvent: WheelEventHandler<HTMLDivElement> = _.noop;
48-
let handleDoubleClick: React.MouseEventHandler<HTMLDivElement> = _.noop;
96+
4997
const wheelStep = getDiffPerSliderStep(sliderRange, wheelFactor, ensuredStep);
5098

51-
handleDoubleClick = (event) => {
99+
const handleDoubleClick: React.MouseEventHandler<HTMLDivElement> = (event) => {
52100
if (
53101
event.target instanceof HTMLElement &&
54102
event.target.className.includes("ant-slider-handle") &&
55103
defaultValue != null
56104
)
57-
// @ts-ignore Argument of type 'number | number[]' is not assignable to parameter of type 'number'.
58-
//TypeScript doesn't understand that onChange always takes the type of defaultValue.
59-
onChange(defaultValue);
105+
if (onResetToDefault != null) {
106+
onResetToDefault();
107+
} else {
108+
// @ts-ignore Argument of type 'number | number[]' is not assignable to parameter of type 'number'.
109+
//TypeScript doesn't understand that onChange always takes the type of defaultValue.
110+
onChange(defaultValue);
111+
}
60112
};
61113

62-
// differentiate between single value and range slider
63-
if (range === false || range == null) {
64-
if (!onWheelDisabled) {
65-
handleWheelEvent = (event) => {
66-
const newValue = value - getWheelStepFromEvent(ensuredStep, event.deltaY, wheelStep);
67-
const clampedNewValue = clamp(min, newValue, max);
68-
onChange(clampedNewValue);
69-
};
70-
}
71-
} else if (range === true || typeof range === "object") {
72-
if (!onWheelDisabled) {
73-
handleWheelEvent = (event) => {
74-
const diff = getWheelStepFromEvent(ensuredStep, event.deltaY, wheelStep);
75-
const newLowerValue = Math.round(value[0] + diff);
76-
const newUpperValue = Math.round(value[1] - diff);
77-
const clampedNewLowerValue = clamp(min, newLowerValue, Math.min(newUpperValue, max));
78-
const clampedNewUpperValue = clamp(newLowerValue, newUpperValue, max);
79-
onChange([clampedNewLowerValue, clampedNewUpperValue]);
80-
};
81-
}
82-
}
83-
84114
return (
85-
<div onWheel={handleWheelEvent} onDoubleClick={handleDoubleClick} style={{ flexGrow: 1 }}>
115+
<div
116+
ref={sliderRef}
117+
onDoubleClick={handleDoubleClick}
118+
style={{ flexGrow: 1, touchAction: "none", userSelect: isFocused ? "none" : "auto" }}
119+
onFocus={() => (isFocused.current = true)}
120+
onBlur={() => (isFocused.current = false)}
121+
>
86122
<AntdSlider {...props} />
87123
</div>
88124
);

frontend/javascripts/oxalis/view/components/setting_input_views.tsx

+6
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,11 @@ export class LogSliderSetting extends React.PureComponent<LogSliderSettingProps>
194194
return Math.round(scaleValue);
195195
};
196196

197+
resetToDefaultValue = () => {
198+
if (this.props.defaultValue == null) return;
199+
this.onChangeInput(this.props.defaultValue);
200+
};
201+
197202
render() {
198203
const { label, roundTo, value, min, max, disabled, defaultValue } = this.props;
199204
return (
@@ -210,6 +215,7 @@ export class LogSliderSetting extends React.PureComponent<LogSliderSettingProps>
210215
value={this.getSliderValue()}
211216
disabled={disabled}
212217
defaultValue={defaultValue}
218+
onResetToDefault={this.resetToDefaultValue}
213219
/>
214220
</Col>
215221
<Col span={this.props.spans[2]}>

frontend/javascripts/oxalis/view/left-border-tabs/layer_settings_tab.tsx

+7-6
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,6 @@ import {
9090
setNodeRadiusAction,
9191
setShowSkeletonsAction,
9292
} from "oxalis/model/actions/skeletontracing_actions";
93-
import {
94-
invertTransform,
95-
transformPointUnscaled,
96-
} from "oxalis/model/helpers/transformation_helpers";
9793
import { addLayerToAnnotation, deleteAnnotationLayer } from "oxalis/model/sagas/update_actions";
9894
import { Model } from "oxalis/singletons";
9995
import { api } from "oxalis/singletons";
@@ -134,6 +130,7 @@ import {
134130
defaultDatasetViewConfigurationWithoutNull,
135131
getDefaultLayerViewConfiguration,
136132
} from "types/schemas/dataset_view_configuration.schema";
133+
import { getSpecificDefaultsForLayer } from "types/schemas/dataset_view_configuration_defaults";
137134
import { userSettings } from "types/schemas/user_settings.schema";
138135
import { confirmAsync } from "../../../dashboard/dataset/helper_components";
139136
import Histogram, { isHistogramSupported } from "./histogram_view";
@@ -1014,8 +1011,12 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps, State> {
10141011
"Opacity"
10151012
);
10161013

1017-
const defaultLayerViewConfig = getDefaultLayerViewConfiguration();
10181014
const isHistogramAvailable = isHistogramSupported(elementClass) && isColorLayer;
1015+
const layerSpecificDefaults = getSpecificDefaultsForLayer(
1016+
this.props.dataset,
1017+
layerName,
1018+
isColorLayer,
1019+
);
10191020

10201021
return (
10211022
<div key={layerName} style={style} ref={setNodeRef}>
@@ -1042,7 +1043,7 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps, State> {
10421043
max={100}
10431044
value={layerConfiguration.alpha}
10441045
onChange={_.partial(this.props.onChangeLayer, layerName, "alpha")}
1045-
defaultValue={defaultLayerViewConfig.alpha}
1046+
defaultValue={layerSpecificDefaults.alpha}
10461047
/>
10471048
{isColorLayer
10481049
? this.getColorLayerSpecificSettings(layerConfiguration, layerName)

frontend/javascripts/types/schemas/dataset_view_configuration_defaults.ts

+8-5
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,13 @@ const eliminateErrors = (
3030
});
3131
};
3232

33-
export const getSpecificDefaultsForLayers = (dataset: APIDataset, layer: APIDataLayer) => ({
34-
intensityRange:
35-
layer.category === "color" ? getDefaultValueRangeOfLayer(dataset, layer.name) : undefined,
36-
alpha: layer.category === "color" ? 100 : 20,
33+
export const getSpecificDefaultsForLayer = (
34+
dataset: APIDataset,
35+
layerName: string,
36+
isColorLayer: boolean,
37+
) => ({
38+
intensityRange: isColorLayer ? getDefaultValueRangeOfLayer(dataset, layerName) : undefined,
39+
alpha: isColorLayer ? 100 : 20,
3740
});
3841

3942
export function ensureDatasetSettingsHasLayerOrder(
@@ -71,7 +74,7 @@ export const enforceValidatedDatasetViewConfiguration = (
7174
const dataset: APIDataset = maybeUnimportedDataset;
7275
dataset.dataSource.dataLayers.forEach((layer) => {
7376
const layerConfigDefault = getDefaultLayerViewConfiguration(
74-
getSpecificDefaultsForLayers(dataset, layer),
77+
getSpecificDefaultsForLayer(dataset, layer.name, layer.category === "color"),
7578
);
7679
const existingLayerConfig = layers[layer.name];
7780

0 commit comments

Comments
 (0)