Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for custom Box rendering #472

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Matchlighter
Copy link

Adds a unsafeDirectRender prop to <Box />. unsafeDirectRender accepts a function with the signature (x: number, y: number, node: DOMNode, output: Output) => void (the same as the internal renderBorder() function). When rendering, Ink will then call the function before rendering Borders or Children.

This is a useful escape hatch for a number of possible components, especially for creating custom "base" components such as (Vertical|Horizontal)Rule. Such components were "possible" using a useEffect hook and measureElement, but suffered from 2 critical issues: 1) resizing the terminal would cause de-syncs, and 2) such an approach requires 2 renders and a short "flash of unstyled content".

@vadimdemedes
Copy link
Owner

I would love to avoid adding user-facing functionality that exposes Ink internals, like Yoga nodes, for example.

Could you post some code that demonstrates the issues you mentioned in the second paragraph?

@Matchlighter
Copy link
Author

@vadimdemedes Sorry I missed your reply - an unnoticed bad filter started filtering my GH notifications incorrectly. Here's a code sample of the problem I'm trying to address:

import useStdoutDimensions from 'ink-use-stdout-dimensions';

export const useMeasureCallback = (measure: 'width' | 'height', cb: (value: number) => void) => {
    const ref = useRef();
    const callback = useRef<typeof cb>();
    callback.current = cb;
    const [columns, rows] = useStdoutDimensions(); // Used to invalidate and trigger a recompute.

    useEffect(() => {
        const meas = measureElement(ref.current);
        callback.current(meas[measure])
    }, [measure == 'height' ? rows : columns]);

    return ref;
}

export const useMeasure = (measure: 'width' | 'height') => {
    const [value, setValue] = useState<number>(0);
    const ref = useMeasureCallback(measure, (v) => {
        setValue(v);
    })

    if (ref.current) {
        return [ref, measureElement(ref.current)[measure]] as const
    }

    return [ref, value] as const;
}

export const VR: React.FC<{ char?: string }> = ({ char }) => {
    const [watchRef, height] = useMeasure('height');

    return <Box width={1} height="100%" ref={watchRef} flexGrow={0} flexShrink={0} flexDirection="column">
        <Text>{(char || '│').repeat(height)}</Text>
    </Box>
}

export const HR: React.FC<{ char?: string }> = ({ char }) => {
    const [watchRef, width] = useMeasure('width');

    return <Box height={1} width="100%" ref={watchRef} flexGrow={0} flexShrink={0}>
        <Text>{(char || '-').repeat(width)}</Text>
    </Box>
}

When used, the <VR /> and <HR /> components are somewhat problematic for the 2 reasons noted in the original post - the {(char || '-').repeat(width|height)} bits are often wrong after a terminal resize, and even in the standard case, they require two renders - the first to do the layout, the second to draw the separator (since width/height aren't valid during the first render).

I understand not wanting to expose internals since it can massively expand the API and can lead to more breaking changes, but I feel this adds a lot of potential with pretty low impact - Yoga should be a consistent API, and the only additional Ink internal exposed is Output. Escape-hatches like this can really help down-stream developers that write addons or even end-products.

@vadimdemedes
Copy link
Owner

Not sure I still get it. Is measureElement API not enough for your use case? Combine this with useEffect hook and you have always up-to-date element dimensions.

@Matchlighter
Copy link
Author

Using that approach has a couple of issues:

  1. It causes multiple renders. Ink renders the layout once (so before useEffect has been triggered) w/o any data on width/height. useEffect then runs, gets a measurement and sets it to some state, and then rendering is triggered a second time. Rendering twice is both a performance impact and causes a short flash of the first render (before measurements have been taken).
  2. In a handful of cases (eg if I resize the terminal within the example above) things get de-synced - my terminal will be 160 columns wide, but measureElement will return 158 - presumably because the change hasn't full propagated through the "DOM" (borrowing terminology from the equivalent in a react-dom environment)

@AlCalzone
Copy link
Contributor

AlCalzone commented Mar 13, 2023

I'm having a similar issue like Matchlighter while trying to build line components. Sometimes the size doesn't propagate until the next layout change or render and explicitly setting the size can prevent it from automatically getting smaller, causing layout issues.

I thought about adding a built-in component to ink, similar to Box but one-dimensional.

@Ruth678
Copy link

Ruth678 commented Sep 11, 2023

Hi,
I've encountered a situation reminiscent of Matchlighter's experience when attempting to construct line components. Occasionally, the size adjustment doesn't take effect until the subsequent layout alteration or rendering, and explicitly defining the size can hinder its automatic reduction, resulting in layout complications.
It crossed my mind to incorporate an integrated element into the ink framework, akin to the best custom boxes but tailored for one-dimensional purposes."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants