feat: Show typedefs on canvas #863
Conversation
georgefst
force-pushed
the
georgefst/canvas-typedefs
branch
from
31b594e to
74bf8a8
Compare
georgefst
force-pushed
the
georgefst/canvas-typedefs
branch
from
74bf8a8 to
08e5f57
Compare
georgefst
force-pushed
the
georgefst/canvas-typedefs
branch
from
08e5f57 to
46e6407
Compare
brprice
force-pushed
the
georgefst/canvas-typedefs
branch
from
46e6407 to
2305e94
Compare
georgefst
force-pushed
the
georgefst/canvas-typedefs
branch
from
2305e94 to
e96d3dc
Compare
georgefst
added
Deploy service
georgefst
added
Deploy service
|
This PR is now successfully deployed, after I added the label again. I think I know why this sometimes doesn't work correctly. I'll work on a fix this weekend. I don't think it'll be completely bulletproof due to all the moving parts, but if my hunch is right, it should be significantly less flaky. |
georgefst
added
Deploy service
georgefst
force-pushed
the
georgefst/canvas-typedefs
branch
from
0e70c49 to
7d911e0
Compare
georgefst
force-pushed
the
georgefst/canvas-typedefs
branch
from
63831de to
a38c198
Compare
georgefst
force-pushed
the
georgefst/canvas-typedefs
branch
2 times, most recently
from
6011e4e to
58cc057
Compare
georgefst
added
Deploy service
This ensures that we _can't_ use any data from the node, instead of treating the data as `any`, which is the default type parameter for `RFNode` and `RFEdge`. Signed-off-by: George Thomas <[email protected]>
Signed-off-by: George Thomas <[email protected]>
We separate out an initial `Def -> Tree` step, then do most of the interesting processing on `Tree`s. This largely unifies `TreeReactFlow` and `TreeReactFlowOne`. It will also prevent a third duplication for typedefs, and maybe more for future features such as previews. Signed-off-by: George Thomas <[email protected]>
This brings in support for displaying and editing type definitions. We need to make quite significant changes to accommodate this. Signed-off-by: George Thomas <[email protected]>
georgefst
force-pushed
the
georgefst/canvas-typedefs
branch
from
58cc057 to
f4b3532
Compare
georgefst
added
Deploy service
| selected: deepEqualTyped(p.selection, { | ||
| tag: "SelectionDef", | ||
| contents: { def: def.name }, | ||
| }), |
There was a problem hiding this comment.
We should actually be able to refactor this sort of thing so that we get selected from makeSelectionFromNode, which would would reduce a decent amount of duplication. We'd need to create just enough of the node, then wrap it with the data.selected value. I just don't know how to do this in a way that TypeScript is happy with. Something vaguely like:
const mkNode = (
n: Exclude<PrimerNode, { data: { selected: boolean } }>,
s: Selection
): PrimerNode => ({
data: { selected: deepEqualTyped(makeSelectionFromNode(n), s) },
...n,
});| @@ -259,6 +264,105 @@ const nodeTypes = { | |||
| {handle("source", Position.Bottom)} | |||
| </> | |||
| ), | |||
| "primer-typedef-name": ({ | |||
There was a problem hiding this comment.
This has all become pretty repetitive. We should find a way to factor out the common parts. But for now, while we haven't decided how to style everything, this actually gives us a lot of flexibility.
| * but only in places where the backend allows an arbitrarily nested (term or type) expression. | ||
| * These are: the bodies and signatures of term defs, and the types of constructor fields in type defs. | ||
| * */ | ||
| export type NodeData = |
There was a problem hiding this comment.
This could maybe do with a better name, but I'm out of ideas.
| @@ -355,7 +459,8 @@ const makePrimerNode = async ( | |||
| p: NodeParams, | |||
| layout: LayoutParams, | |||
| zIndex: number, | |||
| nodeType: NodeType | |||
| nodeData: NodeData, | |||
| def?: GlobalName | |||
There was a problem hiding this comment.
We would ideally modify types somehow to represent that we always expect this to be present for "normal" trees (i.e. not eval), and leaving it off means no selection will be indicated.
| : {}), | ||
| }, | ||
| (parentId) => ({ | ||
| id: JSON.stringify([parentId, id]), |
There was a problem hiding this comment.
We should factor out using JSON.stringify([source,target]) for edge IDs. This was duplicated a bit before this PR, but it's now a lot worse.
dhess
added
Deploy service
Styling here is very boring for now, and doesn't even really distinguish between type and term defs. There was some mostly-orthogonal visual stuff in an earlier version of this branch, but that's been force-pushed away and well be picked up in a fresh PR. Ditto, showing typedefs in a separate row to terms, since we've recently talked about changing the way we pack definitions more generally.
The tree refactor is a change that I've talked about doing for a while (#644 (comment), #644 (comment), #826 (comment)).
The comments below are all small things that I'm happy to leave for future work.