skohub-io / skohub-vocabs

A lightweight tool to publish SKOS Vocabularies
https://skohub.io/
Apache License 2.0
34 stars 25 forks source link

Add JSON-LD links #119

Closed literarymachine closed 3 years ago

literarymachine commented 3 years ago

Fixes #111

acka47 commented 3 years ago

@dr0i, please deploy to test.

dr0i commented 3 years ago

Please resolve conflicts before deployment.

acka47 commented 3 years ago

Conflicts are resolved. @dr0i, please deploy to test.

dr0i commented 3 years ago

Deployed to test.

acka47 commented 3 years ago

I just build the test repo anew, see https://test.skohub.io/acka47/hcrt/heads/master/w3id.org/kim/hcrt/scheme.de.html. Currently, the links are not pointing to the correct URL and I also suggest to put them in the top-right corner, maybe in a line above the heading. I will try to add a mockup on Monday.

acka47 commented 3 years ago

This is where I would like to see the link, a little bit less prominent:

json-ld-link-mockup

dr0i commented 3 years ago

Assigning @sroertgen as reviewer, as discussed. Also mind resolving conflicts.

sroertgen commented 3 years ago

@acka47 When we redeploy this branch on test, could you please tell me, where the link is pointing to and where it should point to? I could not reproduce this locally.

But I suggest testing #117 first. After this is ok, I can pull in master here. Pulling in master now to resolve conflicts would result in having these branches mixed up till master still has to be tested.

acka47 commented 3 years ago

@sroertgen before deploying to test, conflicts have to be resolved.

sroertgen commented 3 years ago

@literarymachine resolved conflicts. we can now deploy to test so @acka47 might give a hint, where the link is pointing to and where it should point to.

acka47 commented 3 years ago

@dr0i please deploy to test.

dr0i commented 3 years ago

Deployed to test.

acka47 commented 3 years ago

@sroertgen See e.g. https://test.skohub.io/acka47/hcrt/heads/master/w3id.org/kim/hcrt/diagram.de.html: the JSON-LD link points to https://test.skohub.io/w3id.org/kim/hcrt/diagram.json instead of https://test.skohub.io/acka47/hcrt/heads/master/w3id.org/kim/hcrt/diagram.json.

sroertgen commented 3 years ago

Hey @acka47,

I changed the css and put the Json png in a dedicated component as we are using it in multiple places (Concept and ConceptScheme). I changed to Gatsby Link component. Let's see if this fixes the issue.

acka47 commented 3 years ago

Please deploy to test, @dr0i .

dr0i commented 3 years ago

Deployed alongside #125 to test.

acka47 commented 3 years ago

The linked URL looks good now, links are broken anyway, see e.g. https://test.skohub.io/acka47/hcrt/heads/master/w3id.org/kim/hcrt/diagram.de.html. This looks like the same problem we encountered in https://github.com/skohub-io/skohub-vocabs/issues/79#issuecomment-720953716. @literarymachine resolved it by using HTML instead of Gatsby links, see https://github.com/skohub-io/skohub-vocabs/commit/2584e9b4c0dd067eb2aac0201f2941db1aba060b.

sroertgen commented 3 years ago

@literarymachine resolved it by using HTML instead of Gatsby links, see 2584e9b.

Ok. I reversed to HTML link like originally in 011d88c3fb2060f21aac6ef555299150a46c6194. Let's have another try.

sroertgen commented 3 years ago

I can't assign @literarymachine as reviewer ( I think because he opened the Pull Request). I think he still should have a look at the changes I made.

acka47 commented 3 years ago

@dr0i Is 011d88c3fb2060f21aac6ef555299150a46c6194 deployed on test? The links are still broken so I think it the last commit wasn't deployed yet...

dr0i commented 3 years ago

@acka47 hm, 011d88c was deployed at 22. December last year. You may mean fd73056 which wasn't deployed (see this ticket - it was commited after my test deployment at https://github.com/skohub-io/skohub-vocabs/pull/119#issuecomment-770668131). I will do update and re-deploy now.

dr0i commented 3 years ago

Deployed to test.

acka47 commented 3 years ago

The problem with the Gatsby links is gone but the URLs point to the wrong direction because the "GitHub bit" (in my test repo:acka47/hcrt/heads/master) is missing. See e.g. https://test.skohub.io/acka47/hcrt/heads/master/w3id.org/kim/hcrt/diagram.de.html.

sroertgen commented 3 years ago

I just saw, now the link is broken again like here:

@sroertgen See e.g. https://test.skohub.io/acka47/hcrt/heads/master/w3id.org/kim/hcrt/diagram.de.html: the JSON-LD link points to https://test.skohub.io/w3id.org/kim/hcrt/diagram.json instead of https://test.skohub.io/acka47/hcrt/heads/master/w3id.org/kim/hcrt/diagram.json.

@literarymachine do you have an idea here? I guess the Gatsby Link thing is not working, because Gatsby is not building the .json-pages and therefore does not know them, so thinks they are not there. Not using the Gatsby Link component the path is not build correctly. Have you experienced this before?

literarymachine commented 3 years ago

When a vocab is built by the webhook server, the base url has to be set as in https://github.com/skohub-io/skohub-vocabs/blob/ded957fa3bc531814f63614e2f4b7743e3904381/src/components/index.js#L15 (but in our case using a plain a instead of the Link component. You can test this by passing the base url as an environment variable:

BASEURL=/acka47/hcrt/heads/master npm run build
sroertgen commented 3 years ago

@dr0i let's have another try.

@literarymachine (process.env.BASEURL || '') was somehow not picked up in the src/components/JsonLink.js-component. I got the value by passing the BASEURL from gatsby-node.js to Concept.js and ConceptScheme.js components. It was already defined to be passed in the createPage context (gatsby-node.js), but not picked up in the components.

dr0i commented 3 years ago

Deployed to test.

acka47 commented 3 years ago

+1 The links are finally working, see https://test.skohub.io/acka47/hcrt/heads/master/w3id.org/kim/hcrt/scheme.de.html! Please deploy to production, @dr0i .

dr0i commented 3 years ago

Deployed to production, closing.