RDFLib / prez-ui

BSD 3-Clause "New" or "Revised" License
10 stars 7 forks source link

Updated the concept hierarchy to cater for vocab API changes #85

Closed jamiefeiss closed 1 year ago

jamiefeiss commented 1 year ago

The PropTableView & ConceptComponent components have been updated to reflect the vocabulary API changes in RDFlib/prez#134 following the design outlined in #84.

Resolves #84.

edmondchuc commented 1 year ago

I know this is in draft still but just wanted to point out that on initial review and testing, I think I've found a bug.

To reproduce on IDN's "IDN Themes" vocabulary:

Screenshot 2023-07-27 at 3 29 34 pm

Screenshot 2023-07-27 at 3 29 50 pm

edmondchuc commented 1 year ago

@jamiefeiss looks like the ordering of concepts by casting to string in the SPARQL query (described in this comment https://github.com/RDFLib/prez/pull/134#issuecomment-1652968653) has fixed the issue I mentioned in the last comment above.

edmondchuc commented 1 year ago

I've been thinking from a UX perspective whether clicking on a concept should open in a new tab or open the page in a modal.

Imagine the scenario where a user is browsing a very large vocabulary and they finally find the term they want, either far down the list by clicking "Load more" or expanding terms to a narrower concept. Currently, if the user clicks a concept, it opens the new page. If the user presses the browser back button, it loads them back to the vocabulary page with the concept hierarchy page back in its initial state. It might get annoying really fast if they have to then drill down to where they were at in the concept hierarchy again.

What do you think @jamiefeiss?

edmondchuc commented 1 year ago

@jamiefeiss should we set the default PER_PAGE variable to a larger value? I'm thinking maybe 100?

jamiefeiss commented 1 year ago

Yeah we could have concepts open in a new tab maybe. It would need to be consistent with how other links are opened in other pages though (e.g. resources under a catalog, etc).

For concepts yeah 100 seems less annoying for users. The PER_PAGE variable should be split into 2 config variables - one for concepts and one for regular pagination (e.g. listing vocabs)

edmondchuc commented 1 year ago

Probably only comment is a question on whether we should couple the hasFewChildren comparison to the conceptPerPage config value or make it a separate config value.