6
\$\begingroup\$

Preamble

This is an Alarm Clock app. The way it works is pretty simple: you change the time by pressing the +/- buttons and set the time by pressing the play button. When the time is up, the alarm will ring at which point you can stop it by pressing the pause button. This question is already long as it is so I won't be including everything required to run the application yourself. But if you'd like to do that, the project is hosted on Github and can be found here. A live version of the app is also available and can be found here.

Comments

In the the beginning, from thinking about how the application would look like, I identified five components:

  • a ChangeTimeButton component which would be the +/- buttons for changing the hour and minute;
  • an ArmButton component to set the alarm to ring at the specified time;
  • a Controls component that would contain the ArmButton and ChangeTimeButtons;
  • a Clock component which would simply display the time;
  • an App component that would contain the Clock and Controls components.

Components

After working with this structure for some time, I felt that ChangeTimeButton had become too complicated. It needed to be a button, which means it had to trigger some action after it was pressed; furthermore, it should be possible for the user to hold it in order to change the hour or minute continuously, and it's appearance while being pressed should be different; it would need to handle both mouse and touch events; a sound should play when it was pressed, and so on. All of this, I thought, had more to do with the fact that this was a button (even if it was a specific type of button) than the fact that it would be used to change the time. So I refactored ChangeTimeButton into two components: one concerned with changing the time and another concerned with being a button. I also broke up ArmButton in a similar way.

Something else that made ChangeTimeButton complicated was that, after the alarm was armed, I did not want the user to still be able to change the time, so there would have to be some notion of ChangeTimeButton being off, which would mean that the user would not be able to interact with it, and that it's appearance would need to change in order to convey that idea. Later, I also found out that I needed to prevent more than one ChangeTimeButton from being pressed at the same time, so it was necessary to maintain state about which one was currently being pressed and disallow all others from modifying the time. But this did not require a change in appearance, so I decided I needed to separate the notion of ChangeTimeButton being off from the notion of it being simply disabled.

Another problem I had was that, because the ChangeTimeButtons would be positioned on each side of the ArmButton, I couldn't think of a good way to make a container component for them. The solution I came up with was to use a custom hook that triggers an update on all registered components whenever the shared state changes. This allowed me to access and modify state shared by all instances of ChangeTimeButton without the need for a container component or a context.


Components

↓ App.tsx

import React, { useState, useRef, useCallback } from "react";
import Clock from "@components/Clock";
import Controls from "@components/Controls";
import useConstructor from "@hooks/useConstructor";
import HighResolutionTimer from "@src/HighResolutionTimer";
import { calcTimeUntilAlert, changeTime, getCurrentTime } from "@src/time";
import "./App.scss";

export default function App() {
    const [mode, setMode] = useState<types.AlarmClockMode>("idle");
    const [time, setTime] = useState<types.Time>();
    const timeoutId = useRef<number>();

    useConstructor(() => {
        const json = localStorage.getItem("time");

        let time;
        if (json === null) {
            time = getCurrentTime();
        } else {
            time = JSON.parse(json);
        }

        setTime(time);
    });

    const armButtonCallback = useCallback(() => {
        if (mode === "idle") {
            setMode("armed");

            let delta = calcTimeUntilAlert(time);
            timeoutId.current = window.setTimeout(() => {
                setMode("fired");
            }, delta);

            localStorage.setItem("time", JSON.stringify(time));
        } else {
            setMode("idle");
            clearTimeout(timeoutId.current);
        }
    }, [mode, time])

    const changeTimeButtonCallback = useCallback((type: types.ChangeTimeButtonType) => {
        const f = {
            "h+": (time: types.Time) => changeTime(time,  1,  0),
            "h-": (time: types.Time) => changeTime(time, -1,  0),
            "m+": (time: types.Time) => changeTime(time,  0,  1),
            "m-": (time: types.Time) => changeTime(time,  0, -1)
        }[type];
        setTime(time => f(time));
    }, []);

    return (
        <div className="outerContainer">
            <div className="innerContainer">
                <Clock time={time} />
                <Controls
                    mode={mode}
                    armButtonCallback={armButtonCallback}
                    changeTimeButtonCallback={changeTimeButtonCallback}
                />
            </div>
        </div>
    );
}

↓ Clock.tsx

import React from "react";
import { formatTime } from "@src/time";
import "./Clock.scss";

type PropsType = { time: types.Time };

export default function Clock(props: PropsType) {
    return (
        <div className="Clock">
            <span className="Clock_fg">{formatTime(props.time)}</span>
            <span className="Clock_bg">88:88</span>
        </div>
    );
};

↓ Controls.tsx

import React, { useEffect, useMemo } from "react";
import ArmButton from "@components/ArmButton";
import ChangeTimeButton from "@components/ChangeTimeButton";
import { useClasses, serializeClasses } from "./useClasses";
import "./Controls.scss";

type PropsType = {
    mode: types.AlarmClockMode;
    armButtonCallback: () => void;
    changeTimeButtonCallback: (type: types.ChangeTimeButtonType) => void;
};

export default function Controls(props: PropsType) {
    const {mode, armButtonCallback, changeTimeButtonCallback} = props;

    const [classes, setClasses] = useClasses();

    const isNotIdle = mode !== "idle";
    useEffect(() => setClasses({Controls__isNotIdle: isNotIdle}), [isNotIdle]);

    return (
        <div className={serializeClasses(classes)}>
            <ChangeTimeButton
                callback={changeTimeButtonCallback}
                off={isNotIdle}
                type="h+"
                className="ChangeTimeButton__left"
            />
            <ChangeTimeButton
                callback={changeTimeButtonCallback}
                off={isNotIdle}
                type="h-"
                className="ChangeTimeButton__left"
            />
            <ArmButton
                callback={armButtonCallback}
                mode={mode}
            />
            <ChangeTimeButton
                callback={changeTimeButtonCallback}
                off={isNotIdle}
                type="m-"
                className="ChangeTimeButton__right"
            />
            <ChangeTimeButton
                callback={changeTimeButtonCallback}
                off={isNotIdle}
                type="m+"
                className="ChangeTimeButton__right"
            />
        </div>
    );
}

↓ ChangeTimeButton.tsx

import React, { memo, useMemo, useCallback } from "react";
import HoldableButton from "@components/HoldableButton";
import { PlusIcon, MinusIcon } from "./icons";
import usePressed from "./usePressed";
import ChangeTimeButtonPressAndHoldSoundPath from "./ChangeTimeButtonPressAndHold.mp3";
import "./ChangeTimeButton.scss";

type PropsType = {
    callback: (type: types.ChangeTimeButtonType) => void;
    off: boolean;
    type: types.ChangeTimeButtonType;
    className: string;
};

const ChangeTimeButton = memo((props: PropsType) => {
    const { callback, type, off, className } = props;

    const [pressed, setPressed] = usePressed();
    const disabled = pressed !== null && pressed !== type;

    const onPress   = useCallback(() => { callback(type); setPressed(type) }, [type]);
    const onRelease = useCallback(() => setPressed(null), []);
    const onHold    = useCallback(() => callback(type), [type]);

    const icon = useMemo(() => {
        return (type === "h+" || type === "m+") ? <PlusIcon/> : <MinusIcon/>;
    }, []);

    return (
        <HoldableButton
            onPress={onPress}
            onRelease={onRelease}
            onHold={onHold}
            disabled={disabled}
            off={off}
            sound={ChangeTimeButtonPressAndHoldSoundPath}
            className={`ChangeTimeButton ${className}`}
        >
            {icon}
        </HoldableButton>
    );
});

export default ChangeTimeButton;

↓ HoldableButton.tsx

import React, { memo, useEffect, useRef } from "react";
import useConstructor from "@hooks/useConstructor";
import { useClasses, serializeClasses } from "./useClasses";
import HighResolutionTimer from "@src/HighResolutionTimer";
import AudioManager, { Sound } from "@src/AudioManager";

type PropsType = React.PropsWithChildren<{
    onPress:   Function;
    onRelease: Function;
    onHold:    Function;
    disabled:  boolean;
    off:       boolean;
    sound:     string;
    className: string;
}>;

const HoldableButton = memo((props: PropsType) => {
    const [classes, setClasses] = useClasses();
    useEffect(() => setClasses({HoldableButton__off: props.off}), [props.off]);

    const spanRef   = useRef<HTMLAnchorElement>();
    const timer     = useRef<HighResolutionTimer>();
    const sound     = useRef<Sound>();
    const isPressed = useRef(false);

    useConstructor(() => {
        timer.current = new HighResolutionTimer(110, 400);

        const audioManager = AudioManager.getInstance();
        sound.current = audioManager.createSound(props.sound);
    });

    const press = (e: any) => {
        e.preventDefault();

        if (e.type === "mousedown" && (("buttons" in e && e.buttons !== 1) || ("which" in e && e.which !== 1))) {
            return;
        }

        if (props.off || props.disabled) {
            return;
        }

        isPressed.current = true;
        setClasses("HoldableButton__pressed");

        props.onPress();
        sound.current.play();
        timer.current.setCallback(() => {
            props.onHold();
            sound.current.play();
        });
        timer.current.start();
    };

    const release = (e: any) => {
        e.preventDefault();

        if (!isPressed.current) {
            return;
        }

        isPressed.current = false;
        setClasses("HoldableButton__released");

        props.onRelease();
        timer.current.stop();
    };

    useEffect(() => {
        spanRef.current.addEventListener("touchstart", press, {passive: false});
        spanRef.current.addEventListener("touchend", release, {passive: false});

        return () => {
            spanRef.current.removeEventListener("touchstart", press);
            spanRef.current.removeEventListener("touchend", release);
        }
    }, [props.off, props.disabled]);

    const className = `${serializeClasses(classes)} ${props.className}`;

    return (
        <span
            ref={spanRef}
            onMouseDown={press}
            onMouseUp={release}
            onMouseLeave={release}
            className={className}
        >
            {props.children}
        </span>
    );
});

export default HoldableButton;

↓ ArmButton.tsx

import React, { useEffect, useMemo } from "react";
import BlinkingButton from "@components/BlinkingButton";
import { PlayIcon, PauseIcon } from "./icons";
import { useClasses, serializeClasses } from "./useClasses";
import ArmButtonPressSoundPath from "./ArmButtonPress.mp3";
import ArmButtonBlinkSoundPath from "./ArmButtonBlink.mp3";
import "./ArmButton.scss";

type PropsType = {
    callback: () => void;
    mode: types.AlarmClockMode;
};

export default function ArmButton(props: PropsType) {
    const [classes, setClasses] = useClasses();

    useEffect(() => setClasses({
        ArmButton__isArmed: props.mode !== "idle"
    }), [props.mode]);

    const icon = useMemo(() => {
        return (props.mode === "idle") ? <PlayIcon/> : <PauseIcon/>;
    }, [props.mode]);

    return (
        <BlinkingButton
            onPress={props.callback}
            blinking={props.mode === "fired"}
            pressSound={ArmButtonPressSoundPath}
            blinkSound={ArmButtonBlinkSoundPath}
            className={serializeClasses(classes)}
        >
            {icon}
        </BlinkingButton>
    );
}

↓ BlinkingButton.tsx

import React, { useRef, useEffect } from "react";
import HighResolutionTimer from "@src/HighResolutionTimer";
import AudioManager, { Sound } from "@src/AudioManager";
import useConstructor from "@hooks/useConstructor";
import { useClasses, serializeClasses } from "./useClasses";

type PropsType = React.PropsWithChildren<{
    onPress:    Function;
    blinking:   boolean;
    pressSound: string;
    blinkSound: string;
    className:  string;
}>;

export default function BlinkingButton(props: PropsType) {
    const [classes, setClasses] = useClasses();

    const timer      = useRef<HighResolutionTimer>();
    const pressSound = useRef<Sound>();
    const blinkSound = useRef<Sound>();
    const isLit      = useRef(false);

    const blink = (lit: boolean) => {
        isLit.current = lit;
        setClasses({BlinkingButton__isLit: lit});
    }

    useConstructor(() => {
        timer.current = new HighResolutionTimer(500, 500, () => {
            blink(!isLit.current);
            blinkSound.current.playIf(isLit.current);
        });

        const audioManager = AudioManager.getInstance();
        pressSound.current = audioManager.createSound(props.pressSound);
        blinkSound.current = audioManager.createSound(props.blinkSound);
    });

    useEffect(() => {
        if (props.blinking) {
            timer.current.start();
        } else {
            timer.current.stop();
            blink(false);
        }
    }, [props.blinking]);

    const callback = (e: React.MouseEvent) => {
        props.onPress();
        pressSound.current.play();
    }

    const className = `${serializeClasses(classes)} ${props.className}`;

    return (
        <span
            onClick={callback}
            className={className}
        >
            {props.children}
        </span>
    );
}

Hooks

↓ makeUseClasses.ts

import { useState } from "react";
import { isString, isFunction } from "@src/utils";

type StringSet = Set<string>;
type GroupDictionary = {[key: number]: Set<string>};
type Spec = {[key: string]: {init: boolean; group: number; precedence: number;}};
type SetClassesArgument = string|types.BoolDictionary|UpdateFunction;
type SetClassesFunction = (...args: SetClassesArgument[]) => void;
type UseClassesFunction = (...initialState: string[]) => [StringSet, SetClassesFunction];
type UpdateFunction = (oldState: StringSet) => string|string[]|types.BoolDictionary;
type SerializeClassesFunction = (state: StringSet) => string;

export default function makeUseClasses(spec: Spec): [UseClassesFunction, SerializeClassesFunction] {
    const groups = getGroups(spec, Object.keys(spec));

    const useClasses = (...initialState: string[]): [StringSet, SetClassesFunction] => {
        checkNames(spec, ...initialState);

        const [classes, _setClasses] = useState<StringSet>(() => {
            const defaultState = Object.keys(spec).filter((key: string) => spec[key].init);
            return new Set([...defaultState, ...initialState]);
        });

        const setClasses = (...args: SetClassesArgument[]) => {
            _setClasses((oldState: StringSet) => {
                let [insert, remove] = parseArguments(oldState, ...args);

                checkNames(spec, ...insert);
                checkNames(spec, ...remove);

                const diff = getDiff(groups, getGroups(spec, [...insert]));
                remove = new Set([...remove, ...diff]);

                const newState = new Set([...oldState, ...insert].filter((key: string) => !remove.has(key)));
                return newState;
            });
        };

        return [classes, setClasses];
    }

    const serializeClasses = (state: StringSet) => {
        const sorted = [...state].sort((a: string, b: string) => {
            return spec[a].precedence - spec[b].precedence
        });
        const result = sorted.join(" ");
        return result;
    }

    return [useClasses, serializeClasses];
}

const parseArguments = (oldState: StringSet, ...args: SetClassesArgument[]) => {
    let insert = new Set<string>();
    let remove = new Set<string>();

    for (let x of args) {
        const value = isFunction(x) ? (x as UpdateFunction)(oldState) : x;

        if (isString(value)) {
            const name = value as string;
            insert.add(name);
        } else if (Array.isArray(value)) {
            const names = value as string[];
            insert = new Set([...insert, ...names]);
        } else {
            const dictionary = value as types.BoolDictionary;
            for (let key of Object.keys(value)) {
                (dictionary[key] ? insert : remove).add(key);
            }
        }
    }

    return [insert, remove];
}

const getDiff = (x: GroupDictionary, y: GroupDictionary): StringSet => {
    let result = new Set<string>();

    for (let key of Object.keys(y)) {
        const group = parseInt(key);
        if (group === 0) return;

        const a = x[group];
        const b = y[group];
        const c = [...a].filter((key: string) => !b.has(key));
        result = new Set([...result, ...c]);
    }

    return result;
}

const getGroups = (spec: Spec, keys: string[]) => {
    let result: {[key: number]: Set<string>} = {};

    for (let key of keys) {
        const group = spec[key].group;
        if (group in result) {
            result[group].add(key);
        } else {
            result[group] = new Set([key]);
        }
    }

    return result;
}

const checkNames = (spec: Spec,...names: string[]) => {
    for (let name of names) {
        console.assert(name in spec, `${name} not in spec.`);
    }
}

↓ makeUseGlobal.ts

import { useState, useEffect } from "react";

export default function makeUseGlobal<T>(initialState: T) {
    let globalState = initialState;
    const listeners = new Set();

    const setState = (value: T) => {
        globalState = value;
        listeners.forEach((listener: Function) => {
            listener();
        });
    }

    return (): [T, Function] => {
        const [state, _setState] = useState<T>(globalState);

        useEffect(() => {
            const listener = () => {
                _setState(globalState);
            }
            listeners.add(listener);
            listener();

            return () => {
                return () => listeners.delete(listener);
            }
        }, []);

        return [state, setState];
    }
}

↓ useConstructor.ts

import { useRef } from "react";

export default function useConstructor(callback: Function, args: any[] = []) {
    const hasBeenCalled = useRef(false);
    if (hasBeenCalled.current) {
        return;
    } else {
        callback(...args);
        hasBeenCalled.current = true;
    }
}

Misc

↓ HighResolutionTimer.ts

type Callback = (timer: HighResolutionTimer) => void;

export default class HighResolutionTimer {
    callback:    Callback;
    duration:    number;
    delay:       number;
    startTime:   number|undefined;
    currentTime: number|undefined;
    timeoutId:   number|undefined;
    totalTicks:  number = 0;
    deltaTime:   number = 0;

    constructor(duration: number, delay: number = 0, callback: Callback = null) {
        this.duration = duration;
        this.delay = delay;
        this.callback = callback;
    }

    reset() {
        this.startTime = this.currentTime = this.timeoutId = undefined;
        this.totalTicks = this.deltaTime = 0;
    }

    tick() {
        const lastTime = this.currentTime;
        this.currentTime = Date.now();

        if (this.startTime === undefined) {
            this.startTime = this.currentTime;
        }

        if (lastTime !== undefined) {
            this.deltaTime = this.currentTime - lastTime;
        }

        this.callback(this);

        const nextTick = this.duration - (this.currentTime - (this.startTime + (this.totalTicks * this.duration)));
        this.totalTicks++;

        this.timeoutId = window.setTimeout(() => this.tick(), nextTick);
    }

    start() {
        console.assert(this.callback !== null, "Timer callback was not set.");

        this.reset();

        this.timeoutId = window.setTimeout(() => this.tick(), this.delay);
    }

    stop() {
        if (this.timeoutId !== undefined) {
            clearTimeout(this.timeoutId);
            this.timeoutId = undefined;
        }
    }

    setCallback(callback: Callback) {
        this.callback = callback;
    }
}

↓ AudioManager.ts

type BufferInfo = {
    buffer: AudioBuffer|null,
    path: string;
    ready: boolean;
};

export default class AudioManager {
    static instance: AudioManager = null;
    context: AudioContext;
    buffers: {[path: string]: BufferInfo} = {};

    constructor() {
        this.context = new (window.AudioContext || window.webkitAudioContext)();
        this.buffers = {};
    }

    static getInstance() {
        if (AudioManager.instance === null) {
            AudioManager.instance = new AudioManager();
        }

        return AudioManager.instance;
    }

    load(bufferInfo: BufferInfo) {
        const request = new XMLHttpRequest();
        request.open("GET", bufferInfo.path);
        request.responseType = "arraybuffer";
        request.onload = () => {
            this.context.decodeAudioData(request.response, (buffer: AudioBuffer) => {
                bufferInfo.buffer = buffer;
                bufferInfo.ready = true;
            });
        }
        request.send();
    }

    createSound(url: string) {
        if (url in this.buffers) {
            return new Sound(this, this.buffers[url]);
        }

        const bufferInfo: BufferInfo = {buffer: null, path: url, ready: false};
        this.buffers[url] = bufferInfo;

        this.load(bufferInfo);

        return new Sound(this, bufferInfo);
    }
}

export class Sound {
    manager: AudioManager;
    bufferInfo: BufferInfo;
    currentSource: AudioBufferSourceNode;

    constructor(manager: AudioManager, bufferInfo: BufferInfo) {
        this.manager = manager;
        this.bufferInfo = bufferInfo;
        this.currentSource = null;
    }

    play() {
        if (!this.bufferInfo.ready) {
            return
        }

        if (this.currentSource !== null) {
            this.currentSource.stop();
        }

        const context = this.manager.context;
        this.currentSource = context.createBufferSource();
        this.currentSource.buffer = this.bufferInfo.buffer;
        this.currentSource.connect(context.destination);
        this.currentSource.start();
    }

    playIf(condition: boolean) {
        if (condition) {
            this.play()
        }
    }
}
\$\endgroup\$
0

1 Answer 1

2
+100
\$\begingroup\$

I have the strong impression that your code can be simplified drastically. I would love to check it out and give it a try. Don't know if I will find the (spare-)time though.

UPDATE: I forked it and currently browse through your code and see what I can simplify. I'm impressed by how clean it is. This is really an astonishingly well written piece of software. (And I learn a good deal in the process. Didn't know that I could use webpack serve and always used webpack-dev-server on the commandline.)

Here is a list of things that I think needs fixing. I will amend the list as I progress through the code:

  • Your makeUseClasses seems to cover the functionality of classnames which is the de-factor standard package for this kind of classname fiddelydoo as far as I can tell from (comercial) React projects I have worked on

  • Your alarm clock functionality could probably be completely hidden away in a single custom hook with a signature something like:

const { incHours, decHours, incMinutes, decMinutes, arm, time } = useAlarmClock();
  • Your useConstructor seems like you are unaware of the more idomatic:
useEffect( () => callOnFirstRenderAndNevermore(), [] );
  • if you only use your useConstructor to initialize state you should better use the built-in initializer capability of the useState hook:
const initTime = () => {
    const json = localStorage.getItem("time");
    let time;
    if (json === null) {
        time = getCurrentTime();
    } else {
        time = JSON.parse(json);
    }
    return time;
};

const [time, setTime] = useState<types.Time>(initTime);
  • you use a string based type for type AlarmClockMode = "idle"|"armed"|"fired"; and I think an enum type is cleaner export enum AlarmClockMode { IDLE, ARMED, FIRED }

  • I would ditch the default exports and switch to named exports completely. This is also the advice given by popular linters and the reasoning behind it is sensible. I experienced it in my work projects myself. The reasons: a named export must be imported by that name, so it is way easier to find all usages with a full-text search reliably. As soon as you notice that you must add something to the file that also needs to be available outside that file (for example an option type for your component) you must introduce named exports, and requires to change the syntax of all places where an import happens. This can be avoided if you use named exports from the beginning.

  • I would introduce a Type for your functional components that specifies the prop type and additionally allows the intrinsic properties (className, tabindex, aria-, data-, etc.)

type ArmButtonProps = {
    callback: () => void;
    mode: AlarmClockMode;
};

export const ArmButton: HTMLAttributesFunctionComponent<ArmButtonProps> = ({
    callback, mode, className, tabindex,
}) => {
  return (
    <div className={classnames('arm-button', className)} tabindex={tabindex} etc.

  • You use @svgr/webpack which is wonderful because it allows to import any SVG and directly use it as a React component with full className and intrinsic attribute support. I feel it complicates matters unnecessary when you wrap it in an Icon component of your own.

  • You really don't need to optimize the resulting JSX because React is very good at comparing your previous result with your current result when render is called and only updates the parts of the DOM that actually changed (that is one reason for the key property in iterated items). So what you are doing here is doing more harm than good:

const icon = useMemo(() => {
        return (mode === AlarmClockMode.IDLE) ? <PlayIcon className="icon" /> : <PauseIcon className="icon" />;
    }, [mode]);
  • The on/off switching of a css class for the blinking animation is better done by using a css based animation. just activate the class an the element and let the browser css rendering do the rest. You can even define the interval length in an SCSS file and from there export it and import it in your js code so that the value is not suplicated throughout the source code and can be changed in one single place.

  • Currently only an idea: I think it might be worth the try to develop a custom hook for the long-button-press behavior. Generally I'm quite in favor of moving functionality into hooks instead of creating specialized components.

  • The file path for the sound files is currently propagated through props, but as it is information that is available during compile time I think this might be a good candidate to move it out of the component code and into some form of export const from some configuration.js

  • more to come ;)

I have pushed some of the proposed changes to my forked repo, so you can see what I have done, but I will stop for now and proceed later.

\$\endgroup\$
1
  • \$\begingroup\$ If you'd like, we can talk in chat. \$\endgroup\$
    – bzrr
    Commented Mar 5, 2021 at 19:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.