-
Notifications
You must be signed in to change notification settings - Fork 0
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 new UI client #37
Conversation
Could you update the README to help build/use the new UI? |
js-client/README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
I added instruction about how to run it locally. Once the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
This is a good PR to scaffold the new frontend SPA.
Did you use a formatter? If not I suggest we use Prettier.
I use prettier. Once we have a |
FYI - @jlesquembre, #39 has now been merged. Also, one question about these changes. From what I can tell, these only include the table and graph views and not the query editor which we have in the prototype deployed to https://nixpkgs-graph.app/. Would it be possible to somehow include that as well (e.g. as a tab)? I think having the table view as the default as you have it here is nice though. |
@dorranh I suggest we don't do that in this PR, get this merged ASAP so we can start on all the UI features we want in smaller, short-lived feature branch. WDYT? |
@GuillaumeDesforges, works for me (we've already made some breaking changes with #39). Let's just make sure that the features in this branch are working as expected - I tried running the client locally and it seemed to be unhappy with the latest version of the API. |
@jlesquembre could you rebase and make sure the UI you implemented works? |
9847650
to
436cd15
Compare
@dorranh could you provide an example to call the |
Hey, you should be able to any valid Gremlin query, for example here is a request JSON body that works for getting {
"query": "g.V()"
} {
"raw": "[]"
} For getting the cytoscape input you need to submit a query like the default one on nixpkgs-graph.app which returns a path, e.g. {
"query": "g.V().filter{it.get().value('pname').matches('auto-multiple-choice')}.repeat(outE().otherV().simplePath()).times(2).path().by('pname').by(label)"
} where That should return an additional attribute called |
Co-authored-by: Guillaume Desforges <[email protected]>
Use `//`, not `#` for comment
436cd15
to
a78f333
Compare
Done, I had to do some changes, now works with the new API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job, thanks a lot 🚀
Let's merge it asap and iterate from there then 😄
Built on top of apiUpdates branch. This PR is only about the JS client, I'm not including any of the changes from
apiUpdates
branchI think it should work with current
main
, but I didn't test it (We want to mergeapiUpdates
anyways, right?)