Reactify cleanup - Githubissues

NLeSC-Storyteller / query-builder-client

A Query builder to query the Knowledge Store and generate datafiles that can be visualized with the Storyteller interface.

Apache License 2.0

0 stars 1 forks source link

Reactify cleanup #27

Closed Maartenvm closed 7 years ago

jspaaks commented 7 years ago

good job intergrating the tree stuff with the rest.

couple of things I noticed:

the import './QueryClearButton.css' should be lowercase file name
the import './QueryBuildButton.css' should be lowercase file name
there's commented out code in a couple of places...a code smell
the search functionality seems broken, at least when I type Agent nothing happens (visualliy at least)
for the record, the veritcal alignment of tree element is not working corretly, but I guess we knew that already. something to do with interaction between React and material design (and superfluous divs)
did you intend to merge back into jhs-reactify?

Maartenvm commented 7 years ago

the import './QueryClearButton.css' should be lowercase file name
the import './QueryBuildButton.css' should be lowercase file name Already fixed, noticed it too today.
there's commented out code in a couple of places...a code smell Can probably be deleted now.
the search functionality seems broken, at least when I type Agent nothing happens (visualliy at least) Works for me... did you update the server you're using to the "add_trigger" branch?
for the record, the veritcal alignment of tree element is not working corretly, but I guess we knew that already. something to do with interaction between React and material design (and superfluous divs)
did you intend to merge back into jhs-reactify? Yes, since this needs to be merged into your code before we can merge all of this back into the master branch

jspaaks commented 7 years ago

about query-builder-server:

master is broken for me
add_trigger branch...don't see it
sqlite_triggers branch also broken

about query-builder-daemon:

master broken for me
sqlite_triggers also broken

I think we should invest some time into writing some documentation.

Maartenvm commented 7 years ago

The master branch of server should work, but it might have been missing a database file in the last commit. I commited just now a version thats working on my machine so ... try again?

The daemon is not strictly necessary to test the client (yet) but that should also work. You do need to change the paths in scripts/storyteller.sh to suit your installation though and install tthe cltl/Storyteller app beside it, so thats rather annoying too. (yes the hardcoded paths are terrible... ask Jason)

jspaaks commented 7 years ago

query-builder-server master branch still gives error, I made an issue here.