-
Notifications
You must be signed in to change notification settings - Fork 30
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 web documentation #201
Conversation
forgot to mention, if you want to run the project locally to give it a try, just checkout the branch, move to the website folder, run If you want to regenerate the dynamic content (values, nodes and examples) just run |
Wow this is so awesome!!! Thanks for doing this. Will take a review soon. |
@aitorllj93 I believe the work extracting the definitions as interfaces #200 will help with the issues around getting the node definitions to generate documentation. |
@aitorllj93 Do we need all these auto-generated pages checked in? Can't they just be made at build time? Or is there a way to build less files? |
a9cf2b2
to
92cbe5c
Compare
@oveddan I removed all of them so now there are less code changes. If you already did checkout of the branch keep in mind I did reset the commit as it was easy to add the ignore before including the files so you might need to reset hard the branch with origin instead of git pull |
Cool, I'll take a look on it and tell you if I find any issue after that PR is merged |
92cbe5c
to
ea51c9b
Compare
@bhouston @oveddan Regarding the dynamic I/O here are the tricks I had to use in order to be able to identify them: First I had to instantiate every single node with numInputs and numOutputs setted so I can get them from the Node instance, also had to initialize the customEvents and variables properties because otherwise some nodes were crashing: That's the reason I think the dynamic I/O specification is incomplete with just the numInputs/numOutputs configuration, I think we should be able to determine before the Node gets instantiated:
Right now this is a must not only to be able to provide a better logic for docs generation but for the visual editor to be able to recognize them as well It would be great to have something like this in the NodeSpec: dynamicInputs:
flows:
valueType: "flow"
keyType: "integer"
maxAmount: 5
minAmount: 1
numbers:
valueType: "integer"
keyType: "text"
maxAmount: 10
dynamicOutputs:
results:
valueType: "text"
keyType: "text"
minAmount: 1 Or maybe considering the current NodeSpec implementation we can extend the inputs & outputs types to allow it: inputs:
- name: flows
valueType: flow
isMultiple: true
keyType: integer
minAmount: 1
maxAmount: 5 And just query by the In order to use them inside the Node we could just get the "group" by the name and iterate over them: const textInputs = readInput<DynamicInput<string, string>[]>('texts');
const inputText = textInputs.find(t => t.name === 'myTextInput').value;
const flowSockets = this.outputSockets.find((s) => s.name === 'flows')?.sockets;
const specificFlowSocket = flowSockets.find(s => s.name === 'mySwitchCase');
|
3269f8a
to
e0eb856
Compare
e0eb856
to
02ec14f
Compare
I tested the automated deployment with GitHub Actions in a personal repo and it works like a charm. @bhouston after the PR gets merged and the gh-pages gets created by the workflow, you'll need to enable it on the repository settings: |
This is phenomenal stuff! Let me merge it and see if I can get it to deploy correctly. |
Looks like the build failed, probably because it wasn't based on the new changes that @oveddan did? https://github.com/bhouston/behave-graph/actions/runs/3817063736 |
Sure it is, I need to update the docs (some of the examples I added to the documentation are using the old way to define nodes) and make the dynamic generation for node profiles working with the new interfaces, I'll try to get some time during next week or weekend to fix it |
@aitorllj93 could you also make sure that you are importing the other packages within import {
registerCoreProfile,
registerSceneProfile,
Registry
} from '@behave-graph/core'; and not using relative paths? You can see how the |
@oveddan after checking the code I noticed some things are failing with the code in the Outdated configuration on the CustomEvent nodes: Error in nodeFactory > makeCommonProps: I'm not sure how to fix the last one, can you please check it? |
@aitorllj93 this has been fixed in #211 |
As discussed here I have been working on improving the docs. Here's a first approach by using Docusaurus
Landing Page:
Core Concepts (+ dark theme which I personally prefer):
Auto-generated pages for Values and Nodes (for Core and Scene profiles):
Auto-generated pages with the Examples found in the graphs folder:
Easy to use blog in case we need it (can be disabled if there's no plans to use it):
That said, there are some troubles I found during the development and should be discussed:
Many different interfaces for Nodes (NodeDescription, NodeDescription2, Node Instance, NodeSpecJSON) and some misalingments between them. It would be nice to be able to generate the Node pages just with the NodeSpecJSON but right now it's not possible:
writeNodeSpecsToJSON
fails when executing on a Registry with just theScene
profile so I need to load 2 registries, to be able to check which Nodes are from each ProfilehelperText
which in this case it's quite usefulThe reactflow component has some issues that makes it hard to reuse it right now:
As we're having many changes on the core interfaces (NodeDescription) and the way we define the nodes, I don't think it makes sense to update the reactflow component till those changes are completed. But once we have polished those details, I would like to improve the visual editor so we can integrate it inside the docs and provide a visual execution environment for the examples section.
Also, we should include a GH Action to deploy the website to GH pages. I can work on that before we merge this PR and include it with the changes.
cc @bhouston @oveddan