-
Notifications
You must be signed in to change notification settings - Fork 25
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
Support for skos:collection #200
Conversation
@sroertgen will do the code review next week, I will do a functional review. |
From a functional view, this already looks very good.
IMO, it remains to be dicussed whether we could/should also include the inverse of |
@@ -28,7 +28,7 @@ const App = ({pageContext, children}) => { | |||
const idx = FlexSearch.create() | |||
// add custom matcher to match umlaute at beginning of string | |||
idx.addMatcher({ | |||
'[Aä]': "a", // replaces all 'ä' to 'a' |
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.
Thanks for that one :)
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.
looks goot to me, just added a key to the li
element in Collection.js
@acka47 you did not approve yet. From my side everything is good. At some point in the future, we might have to think about if we want to display the collection also elsewhere, so users can navigate them. But that is out of scope here |
Yes, because we still have to discuss #200 (comment). What do you think? |
Do you have a specific use-case in mind for this? Since this is no SKOS property I think people are not expecting it and would rather use a One other thing I just noticed: If the |
No, I haven't. I am just concerned about consistency. Until now, everything that you can see in the HTML is also available as structured data. It might lead to confusion when this is changed. On the other hand, adding a relation that is not part of SKOS will also confuse some users. As there is no perfect solution, I am ok with leaving it as is.
I would not assume that a SkoHub Vocabs user is savvy about all the SKOS properties.
Interesting. I assumed |
Yes, I also thought so, but the SKOS-Primer and the SKOS-Reference don't seem to prohibit that a
Well then, I think we are good to go |
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.
+1
@dr0i Please deploy the changes to production. |
Deployed to production. |
This pull request adds support for
skos:collection
as described in #159 (comment)Link from concept to collection:
Collection page linking to all contained concepts:
Since SKOS does not provide an inverse for the
skos:member
relation, collection membership of concepts is determined and stored manually during page creation.Fixes #159