Skip to content
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

Merged
merged 6 commits into from Mar 30, 2023
Merged

Add new UI client #37

merged 6 commits into from Mar 30, 2023

Conversation

jlesquembre
Copy link
Member

Built on top of apiUpdates branch. This PR is only about the JS client, I'm not including any of the changes from apiUpdates branch

I think it should work with current main, but I didn't test it (We want to merge apiUpdates anyways, right?)

@GuillaumeDesforges
Copy link
Contributor

Could you update the README to help build/use the new UI?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@jlesquembre
Copy link
Member Author

Could you update the README to help build/use the new UI?

I added instruction about how to run it locally. Once the apiUpdates branch is merged, I'll add build instructions, I want to add a build script to the flake on that branch.

Copy link
Contributor

@GuillaumeDesforges GuillaumeDesforges left a 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.

.gitignore Outdated Show resolved Hide resolved
js-client/index.html Outdated Show resolved Hide resolved
js-client/package-lock.json Outdated Show resolved Hide resolved
js-client/package.json Outdated Show resolved Hide resolved
js-client/package.json Show resolved Hide resolved
js-client/src/nix-search.ts Show resolved Hide resolved
js-client/src/nix-search.ts Outdated Show resolved Hide resolved
js-client/src/nix-search.ts Outdated Show resolved Hide resolved
js-client/src/nix-search.ts Outdated Show resolved Hide resolved
js-client/src/packages.ts Outdated Show resolved Hide resolved
@jlesquembre
Copy link
Member Author

Did you use a formatter? If not I suggest we use Prettier.

I use prettier. Once we have a flake.nix we could add a command to check the format.

@dorranh
Copy link
Contributor

dorranh commented Mar 28, 2023

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.

@GuillaumeDesforges
Copy link
Contributor

GuillaumeDesforges commented Mar 28, 2023

@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?

@dorranh
Copy link
Contributor

dorranh commented Mar 28, 2023

@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.

@GuillaumeDesforges
Copy link
Contributor

@jlesquembre could you rebase and make sure the UI you implemented works?

@jlesquembre
Copy link
Member Author

@dorranh could you provide an example to call the /gremlin endpoint? With the latest API version I always get an empty response. Looks like something is wrong with my query but I don't find the reason.

@dorranh
Copy link
Contributor

dorranh commented Mar 29, 2023

Hey, you should be able to any valid Gremlin query, for example here is a request JSON body that works for getting raw data (but not the cytoscape graph input)

{
    "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 auto-multiple-choice is the package name.

That should return an additional attribute called cyto_data in the response.

@jlesquembre
Copy link
Member Author

jlesquembre commented Mar 29, 2023

could you rebase and make sure the UI you implemented works?

Done, I had to do some changes, now works with the new API

Copy link
Contributor

@GuillaumeDesforges GuillaumeDesforges left a 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 😄

@jlesquembre jlesquembre merged commit a554041 into main Mar 30, 2023
1 check passed
@jlesquembre jlesquembre deleted the jl/js-client branch March 30, 2023 08:36
This was referenced Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants