Skip to content

Commit

Permalink
fix: changing time configuration with keyboard did not work - #402 (#489
Browse files Browse the repository at this point in the history
)

* refactor: convert onMouse in sliderRnageList to a generalized change handler instead of a mouse event

* refactor: use new handleConfigChange function instead on onMouse event

* refactor: fix comment formatting
  • Loading branch information
ziadevcom authored Nov 12, 2023
1 parent 2e395e0 commit b9b5248
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 15 deletions.
17 changes: 12 additions & 5 deletions app/renderer/src/routes/Config/ConfigSlider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ export type ConfigSliderProps = {
minValue: number;
maxValue: number;
value: number;
onMouseUp?:
| ((event: React.MouseEvent<HTMLInputElement, MouseEvent>) => void)
| undefined;
handleConfigChange?: (value: any) => void;
};

const ConfigSlider: React.FC<ConfigSliderProps> = ({
Expand All @@ -19,12 +17,22 @@ const ConfigSlider: React.FC<ConfigSliderProps> = ({
minValue,
maxValue,
value,
onMouseUp,
handleConfigChange,
}) => {
const [rangeValue, setRangeValue] = useState(value);

const onChange = (e: React.ChangeEvent<HTMLInputElement>) => {
setRangeValue(parseInt(e.target.value));

/*
Check if "handleConfigChange" exists to avoid typescript compile error.
Error comes because type "ConfigSliderProps" is used in "src/config.ts" for "rangeConfig" object. But that object is not used by anything else in the app which means its unnecessary code.
Correct solution would be to make "handleConfigChange" mandatory instead of optional in "ConfigSliderProps" type & remove/disable the "rangeConfig" object from "src/config.ts".
*/

if (handleConfigChange) handleConfigChange(e.target.value);
};

useEffect(() => {
Expand All @@ -43,7 +51,6 @@ const ConfigSlider: React.FC<ConfigSliderProps> = ({
minValue={minValue}
maxValue={maxValue}
onChange={onChange}
onMouseUp={onMouseUp}
/>
</StyledRangeContainer>
);
Expand Down
27 changes: 17 additions & 10 deletions app/renderer/src/routes/Config/SliderSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ const SliderSection: React.FC = () => {
minValue: 1,
maxValue: 120,
value: stayFocus,
onMouseUp: useCallback(
(e) => dispatch(setStayFocus(parseInt(e.target.value))),
handleConfigChange: useCallback(
(value) => dispatch(setStayFocus(parseInt(value))),
[dispatch]
),
},
Expand All @@ -39,8 +39,8 @@ const SliderSection: React.FC = () => {
minValue: 1,
maxValue: 60,
value: shortBreak,
onMouseUp: useCallback(
(e) => dispatch(setShorBreak(parseInt(e.target.value))),
handleConfigChange: useCallback(
(value) => dispatch(setShorBreak(parseInt(value))),
[dispatch]
),
},
Expand All @@ -50,8 +50,8 @@ const SliderSection: React.FC = () => {
minValue: 1,
maxValue: 60,
value: longBreak,
onMouseUp: useCallback(
(e) => dispatch(setLongBreak(parseInt(e.target.value))),
handleConfigChange: useCallback(
(value) => dispatch(setLongBreak(parseInt(value))),
[dispatch]
),
},
Expand All @@ -61,8 +61,8 @@ const SliderSection: React.FC = () => {
minValue: 1,
maxValue: 10,
value: sessionRounds,
onMouseUp: useCallback(
(e) => dispatch(setSessionRounds(parseInt(e.target.value))),
handleConfigChange: useCallback(
(value) => dispatch(setSessionRounds(parseInt(value))),
[dispatch]
),
},
Expand All @@ -72,7 +72,14 @@ const SliderSection: React.FC = () => {
<StyledConfigSliderSection>
{sliderRangeList.map(
(
{ label, valueType, minValue, maxValue, value, onMouseUp },
{
label,
valueType,
minValue,
maxValue,
value,
handleConfigChange,
},
index
) => (
<ConfigSlider
Expand All @@ -81,7 +88,7 @@ const SliderSection: React.FC = () => {
minValue={minValue}
valueType={valueType}
maxValue={maxValue}
onMouseUp={onMouseUp}
handleConfigChange={handleConfigChange}
key={index}
/>
)
Expand Down

0 comments on commit b9b5248

Please sign in to comment.