-
Notifications
You must be signed in to change notification settings - Fork 7
Adding SHACL validation #161
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
base: main
Are you sure you want to change the base?
Conversation
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.
@danielbeeke Thanks for your contribution! I don't have a lot of remarks about the code itself, just some UX considerations/suggestions below
- Could you also add the SHACL report as an asset to the TriplyDB upload?
- Please resolve the conflicts
Thanks again for your contribution!
P.S. @pietervaneverdingen From the video I'm a bit worried about the intuitiveness of the drag and drop functionality for the SHACL bindings with the current UI. Although, since that is not part of this PR I guess something to discuss at a later stage
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.
Is it possible to solve this using CSS's :hover selector? The only issue I can forsee is that you need to be quite specific with the selector?
| if (!report.conforms) { | ||
| const writer = new Writer() | ||
| for (const quad of report.dataset) writer.addQuad(quad) | ||
| writer.end((error, result) => setShaclReportTurtle(result)) |
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.
Could you add a comment that this function is synchronous. In most functions, giving a callback indicates asynchronous functionality
| Download RDF | ||
| </Button> | ||
| </Button> : | ||
| <SplitButton |
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 think UX wise, splitting into two makes a bit more sense, as they download separate results. a SHACL report and the transformed output. The split button approach makes more sense if we're talking about the same product. (Unless I missed something and the SHACL report does include the transformed result)
|
|
||
| {shaclConforms === false ? <span><br /><br /><FontAwesomeIcon icon={["fas", "xmark"]} /> The data does not conform to the selected SHACL shape</span> : null} | ||
| {shaclConforms === true ? <span><br /><br /><FontAwesomeIcon icon={["fas", "check"]} /> The data does conform to the selected SHACL shape</span> : null} |
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.
It might be nice to use the mui Alert component for this. This might also offer a "better" place to allow the user to download the SHACL report (either as an action or as a button in the alert)
Here is the part of adding SHACL validation and the report to the output.
I exposed the report via a split button. The RDF can still be downloaded despite not conforming to the SHACL shape.
Here is a little screen cast showing the functionality
Screencast.from.2023-12-06.12-17-26.webm