flatironinstitute / stan-playground

Run Stan models in the browser
Apache License 2.0
36 stars 1 forks source link

Simplify div absolute positioning and styling #112

Closed magland closed 4 months ago

magland commented 5 months ago

Since we are at a good point for sweeping through the codebase and making changes, I wanted to propose the following for simplifying all the absolute positioning and styling of divs throughout. This doesn't preclude doing a further overhaul down the road, but this should tidy things up a bit.

Put a "Div" component, in the components/ folder defined like this

import { CSSProperties, FunctionComponent, PropsWithChildren } from "react"

type DivProps = {
    left?: number | undefined
    top?: number | undefined
    width?: number | undefined
    height?: number | undefined
    position?: 'absolute' | 'relative'
    xScroll?: boolean
    yScroll?: boolean
    backgroundColor?: string
    style?: CSSProperties
    onKeyDown?: (event: React.KeyboardEvent<HTMLDivElement>) => void
    className?: string
}

const Div: FunctionComponent<PropsWithChildren<DivProps>> = ({
    children,
    left = undefined,
    top = undefined,
    width = undefined,
    height = undefined,
    position = 'absolute',
    xScroll = false,
    yScroll = false,
    backgroundColor = undefined,
    style = {},
    onKeyDown = undefined,
    className = undefined
}) => {
    const divStyle: CSSProperties = {
        left,
        top,
        width,
        height,
        position,
        overflowX: xScroll ? 'scroll' : 'hidden',
        overflowY: yScroll ? 'scroll' : 'hidden',
        backgroundColor,
        ...style
    }
    return (
        <div className={className} style={divStyle} onKeyDown={onKeyDown}>
            {children}
        </div>
    )
}

export default Div

Then for example

<div style={{position: 'absolute', width, height, overflow: 'auto'}}>

Would become

<Div width={width} height={height} yScroll={true}>

and

<div className="left-panel" style={{ position: 'absolute', left: 0, top: topBarHeight + 2, width: leftPanelWidth, height: height - topBarHeight - 2, overflow: 'auto' }}>

would become

<Div className="left-panel" width={leftPanelWidth} height={height - topBarHeight - 2} top={topBarHeight + 2} yScroll={true}>
WardBrian commented 5 months ago

If I’m reading this correctly it sounds like an alternative to #23 and #24, correct?

I think I would prefer using css classes that live outside the code files for as much as possible. Obviously if we need a computed value like a width we’re feeding in from the outside that may be a different story

magland commented 5 months ago

If I’m reading this correctly it sounds like an alternative to #23 and #24, correct?

I think I would prefer using css classes that live outside the code files for as much as possible. Obviously if we need a computed value like a width we’re feeding in from the outside that may be a different story

Right now, css is not possible for handling positioning (left, top, width, height); in the future this could eventually change if we are able to do an overhaul. And I feel like yScroll belongs in the .tsx files because that's integral to the functioning and not really a style. But I could see the case for backgroundColor being in a .css file. What I'm proposing is a simplification of the existing system... and those other issues would still remain open.

WardBrian commented 4 months ago

I think #135 resolved the same issues in a different way