useWindowStore is too complicated and too opinionated; this is causing problems with rendering, and it makes state updates that should be simple more complicated than they need to be.
The main bit of frontend state — windowTree — should be changed to a map of number to string | null. The numbers will represent window IDs, and the strings will be the paths that these windows have open.
I like the way the window manager behaves, where each node has two children, and would like to keep that binary-tree structure. This structure should be implied in the numbering in the map: each number is either n*2 or (n*2)+1 of a previous number in the map, and from that the windowContainer can build a tree of windows and window containers.
A nice-to-have for this PR is fixing the UI bug where splitting a window causes an unsightly flash where the space is empty for a few milliseconds. I think this can be fixed like so:
- Whatever else, avoid a situation where "splitting a window" means unmounting and remounting its parent in the DOM.
- The rendering logic, which will live in
WindowContainer, is simple. For every entry in the map, we have props id and path. If that entry is a node which could have a left and right child, we can infer that the ids of those children will be id*2 and (id*2)+1 respectively. If the path prop is null, we know that this window is not rendering content, but more windows. We don’t want to update the path of this node if we add a child to it, so we just say this: if a node’s path is null, render the window whose ID is id*2 as the left child of this node.
- The default value of the app state is a map with two K-V pairs:
[1, null] and [2, ‘~sampel/home’].
- The above displays one “window” as far as the user is concerned. To “split this window” is to just add a third K-V pair to the map:
[3, ‘~sampel/path’], where “~sampel/path” is just a placeholder for whatever the real path is.
- React will notice this state change and then update the UI accordingly; given the above, it should be possible to do this without visibly unmounting and remounting the existing window at
2 in the map, but I’m not sure exactly what the JSX will look like.
- I suspect you want to render two
Allotments per node, even if the node only has one child. To “split a window” is then to just add an Allotment.Pane to the empty Allotment next to “the window” that we’re seeing.
- This will all still involve a recursive
WindowContainer component and paying attention to the key attribute; maybe the WindowContainer should try to render the n*2 child beneath it even if no such K-V pair exists in the map, and then the WindowContainer trying to render that child needs to render the empty Allotment with that non-existent key so that when a new node is added, React knows that the newly-created Allotment with key 4 is replacing the already-existing Allotment with key 4.
useWindowStore is too complicated and too opinionated; this is causing problems with rendering, and it makes state updates that should be simple more complicated than they need to be.
The main bit of frontend state —
windowTree— should be changed to a map ofnumbertostring | null. Thenumbers will represent window IDs, and thestrings will be the paths that these windows have open.I like the way the window manager behaves, where each node has two children, and would like to keep that binary-tree structure. This structure should be implied in the numbering in the map: each number is either
n*2or(n*2)+1of a previous number in the map, and from that thewindowContainercan build a tree of windows and window containers.A nice-to-have for this PR is fixing the UI bug where splitting a window causes an unsightly flash where the space is empty for a few milliseconds. I think this can be fixed like so:
WindowContainer, is simple. For every entry in the map, we have propsidandpath. If that entry is a node which could have a left and right child, we can infer that theids of those children will beid*2and(id*2)+1respectively. If thepathprop isnull, we know that this window is not rendering content, but more windows. We don’t want to update thepathof this node if we add a child to it, so we just say this: if a node’spathisnull, render the window whose ID isid*2as the left child of this node.[1, null]and[2, ‘~sampel/home’].[3, ‘~sampel/path’], where “~sampel/path” is just a placeholder for whatever the real path is.2in the map, but I’m not sure exactly what the JSX will look like.Allotmentsper node, even if the node only has one child. To “split a window” is then to just add anAllotment.Paneto the emptyAllotmentnext to “the window” that we’re seeing.WindowContainercomponent and paying attention to thekeyattribute; maybe theWindowContainershould try to render then*2child beneath it even if no such K-V pair exists in the map, and then theWindowContainertrying to render that child needs to render the emptyAllotmentwith that non-existentkeyso that when a new node is added, React knows that the newly-createdAllotmentwith key4is replacing the already-existingAllotmentwith key4.