Feature: Improvements to usability of input modals

[mafredri]

• Version: 3.0.0

I tried re-implementing one of my old experiments with the new input modals and found a few shortcomings for that use-case. Even so, I really like what you did with them, nice job!

Here are my findings:

1. Input modals cannot capture focus (always focused until closed) OR a way to do window.focus() without losing modal focus. Essentially an API like window.raise() but more reliable "raise to top" behavior (see demo using .focus()). The window.raise() API often doesn't bring a window all the way up.
   • A workaround is storing Window.focused() after the modal is created, but it's brittle (how do we know it's our input modal vs another modal?) and swapping focus between windows quickly is not very reliable and keyboard input can be leaked to the other window during focus swaps
2. Input modals cannot receive all input (e.g. escape, return, tab, shift+tab), it'd be nice to be able to handle these keys via modal input instead of assigning new key handlers that are disassociated from the modal. If modal loses focus, pressing Enter in another application still causes an action in the modal unless we also start tracking state of windowDidFocus (results in a lot of state handling between windows and key handlers).
3. It'd be nice to be able to change the color of "placeholder" text so that input modals and non-input modal text color can be matched.
   • Also, the placeholder text color when using appearance dark and macOS Light does not differentiate significantly from the non-placeholder text, the above could help
4. Mouse clicks pass through a standard modal, but not an input modal (it makes sense it does not for the latter, but should they behave similarly?)
5. Using the origin function in Modal.build does not propagate to future modal updates which limits its usefulness, it'd be nice if setting an icon on the modal or updating the text could re-trigger the origin function so it becomes e.g. centered
6. Running Window.all({visible: true}) during modal show can result in a janky animation (freezes halfway through), it can also prevent keyboard input until the action completes. I understand this is because JS is all running on the same thread, but is there any opportunities for improvement here? Could we use a Promise based API for functions like Window.all() that can run on a separate thread? Or introduce a Phoenix.thread(() => Window.all()) wrapper? Or maybe just run the input modal on a separate thread, resulting in a lot of calls to textDidChange once the blocker is gone? Ideally these APIs wouldn't even be slow in the first place, but since we're at the mercy of app developers..

Example usage:

Example code (very much a wip):

// phoenix.ts
import {SearchWindows} from './search';

Key.on('m', ['cmd', 'ctrl', 'alt'], (_, repeated) => {
    if (repeated) {
        return;
    }

    const search = new SearchWindows();

    const focusedWindow = Window.focused();
    const screen = Screen.main();

    const origin =
        (offset: number = 0.5) =>
        (frame: Rectangle): Point => {
            const sf = screen.visibleFrame();

            return {
                x: sf.x + sf.width / 2 - frame.width / 2,
                y: sf.y + sf.height / 2 - frame.height * offset,
            };
        };
    const resultOrigin = origin(1.7);
    const inputOrigin = origin();

    const setActiveColor = (modal: Modal) => {
        modal.setTextColor(200, 200, 200, 1);
    };
    const setFadedColor = (modal: Modal) => {
        modal.setTextColor(60, 60, 60, 1);
    };
    const result = Modal.build({
        origin: resultOrigin,
        font: 'Go',
        text: 'Result',
    });
    setFadedColor(result);

    const input = Modal.build({
        origin: inputOrigin,
        font: 'Go',
        appearance: 'dark',
        hasShadow: true,
        isInput: true,
        inputPlaceholder: 'Search',
        textAlignment: 'right', // 'center'
    });
    setActiveColor(input);

    const modals = [result, input];

    const focus = (w: Window | undefined) => {
        w?.focus();
        result.icon = w?.app().icon();
    };
    const cycleKeys = [
        new Key('tab', [], () => focus(search.next())),
        new Key('tab', ['shift'], () => focus(search.previous())),
    ];
    cycleKeys.forEach((k) => k.enable());

    input.textDidChange = (s) => {
        log('Modal input:', s);

        if (!search.search(s)) {
            focusedWindow?.focus();
            result.icon = undefined;
            result.text = 'Results';
            setFadedColor(result);
        } else {
            focus(search.current());
            result.text = `Results: ${search.numMatches()}`;
            setActiveColor(result);
        }
        result.origin = resultOrigin(result.frame());
    };

    // Submit or escape.
    let closeKeys: Key[] = [];
    const close = (key: string) => {
        [...closeKeys, ...cycleKeys].forEach((k) => k.disable());
        modals.forEach((m) => m.close());
        if (key == 'escape') {
            focusedWindow?.focus();
        }
        if (key == 'return') {
            if (search.numMatches() > 0) {
                // Leave focus on current window.
            } else {
                focusedWindow?.focus();
            }
        }
    };
    closeKeys = ['return', 'escape'].map(
        (k) => new Key(k, [], (handler) => close(handler.key)),
    );

    modals.forEach((m) => m.show());
    closeKeys.forEach((k) => k.enable());
    setTimeout(() => search.init(), 250); // Show modal responsively.
});

// search.ts
interface windowEntry {
    win: Window;
    appName: string;
    title: string;
}

export class SearchWindows {
    private prevReverse = true;
    private winCache: windowEntry[] = [];
    private matches: Window[] = [];

    init() {
        this.winCache = Window.all({visible: true}).map((w) => ({
            win: w,
            appName: w.app().name(),
            title: w.title(),
        }));
        this.matches = this.winCache.map((e) => e.win);
    }

    search(s: string): boolean {
        this.prevReverse = true; // Reset.
        this.matches = this.winCache
            .filter(
                (e) =>
                    e.appName.toLowerCase().match(s.toLowerCase()) ||
                    e.title.toLowerCase().match(s.toLowerCase()),
            )
            .map((e) => e.win);
        return s.length > 0 && this.matches.length > 0;
    }

    current() {
        if (this.matches.length) {
            return this.matches[0];
        }
    }

    numMatches() {
        return this.matches.length;
    }

    next() {
        return this.cycle(false);
    }

    previous() {
        return this.cycle(true);
    }

    private cycle(reverse: boolean): Window | undefined {
        if (!this.matches.length) {
            return;
        }

        if (this.prevReverse !== reverse) {
            this.prevReverse = reverse;
            this.cycle(reverse); // Rotate.
        }

        const w = reverse ? this.matches.pop() : this.matches.shift();
        if (!w) {
            return;
        }
        reverse ? this.matches.unshift(w) : this.matches.push(w);
        return w;
    }
}

[kasper]

@mafredri Good feedback, I like it, thanks! 😄 I will try to capture some of these.

[mafredri]

I did a little investigation regarding 1., the reason raise() might not be working in this use-case is that it (raise) does not consider the modal the focused window. Instead it raises the windows behind the window that was focused before I triggered the modal. Interestingly Window.focused() does consider the modal the focused window, so there seems to be a little mis-match in behavior between the two.

[kasper]

Great feedback! 🚀 I did the following:

1. Added Modal#focus() to focus the modal and make it the key window to receive events (552d93c)
2. Added property textDidCommit for callback function when the input modal’s text field is committed, this will capture return, tab and backtab (a5d0315)
3. Split this into #332
4. I think it’s correct behaviour that the input modal allows for mouse events but the standard modal doesn’t
5. Added property didResize for callback function when the modal resizes. Also bound it to the origin function when building a modal to reposition automatically (65b886a).
6. I think this is separate from modal improvements