Autocomplete to support distinguisher labels in case of shared preferred labels

[kouralex]

Reasons for creating this PR

There are vocabularies (such as https://dev.finto.fi/mts/fi/) that require shared preferred labels (e.g., "opettaja") to exists in different parts of the vocabulary. However, in the past it was impossible to distinguish between autocomplete hits, see #1335 for more information.

Link to relevant issue(s), if any

• Closes Autocomplete should be able to support concept scheme/group information #1335

Description of the changes in this PR

This PR bring in the ability to differentiate between identical autocomplete results via special vocabulary-specific skosmos:resultDistinguisher. Only works in case a single vocabulary search is initiated.

Known problems or uncertainties in this PR

• Is UI layout OK?
• It is up to the configuration setter to provide a legit SPARQL property path. E.g., prefixes may or may not be available.

Possible enhancements still to be done

• SPARQL property path could be constructed from specially crafted RDF graph, e.g., similarly as it is done in SHACL
• Relation with skosmos:alphabeticalListQualifier?

Checklist

• phpUnit tests pass locally with my changes
• I have added tests that show that the new code works, or tests are not relevant for this PR (e.g. only HTML/CSS changes)
• The PR doesn't reduce accessibility of the front-end code (e.g. tab focus, scaling to different resolutions, use of .sr-only class, color contrast)
• The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (1d48d64) 71.17% compared to head (b7810f8) 70.20%.
Report is 51 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1380      +/-   ##
============================================
- Coverage     71.17%   70.20%   -0.97%     
- Complexity     1650     1664      +14     
============================================
  Files            32       32              
  Lines          3788     4293     +505     
============================================
+ Hits           2696     3014     +318     
- Misses         1092     1279     +187     

Files	Coverage Δ
model/VocabularyConfig.php	95.51% <100.00%> (+0.03%)	⬆️
model/sparql/GenericSparql.php	92.56% <96.00%> (+1.28%)	⬆️

... and 13 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
1 Security Hotspot
8 Code Smells

No Coverage information
0.0% Duplication

[frosterus]

Three alternatives for the UI:

First option

Adding the distinguisher in parentheses after the autocomplete result but in dark grey so as to differentiate from a prefLabel that contains a part in parentheses. This solution is the least disruptive to the user but might cause confusion if they don't realize the significance of the change in colour (that the prefLabel does not contain the dark grey part). This could be seen as following the approach of altLabels, though so might not be an issue.

Second option

Adding the distinguisher on top of the concept type. A very clear look that should be easy for the user to parse though does extend each result with a distinguisher into two lines.

Third option

Using the symbols from the search result page to show the Property used for disambiguation. The problem is that the symbols are not used elsewhere in Finto and might not be very clear in their meaning. Does not work well if the distinguishing pattern is more complex than a single property. Also the most complex solution to code since additional data needs to be conveyed to the autocomplete.

[MikkoAleksanteri]

Three alternatives for the UI:

First option

Adding the distinguisher in parentheses after the autocomplete result but in dark grey so as to differentiate from a prefLabel that contains a part in parentheses. This solution is the least disruptive to the user but might cause confusion if they don't realize the significance of the change in colour (that the prefLabel does not contain the dark grey part). This could be seen as following the approach of altLabels, though so might not be an issue.

Second option

Adding the distinguisher on top of the concept type. A very clear look that should be easy for the user to parse though does extend each result with a distinguisher into two lines.

Third option

Using the symbols from the search result page to show the Property used for disambiguation. The problem is that the symbols are not used elsewhere in Finto and might not be very clear in their meaning. Does not work well if the distinguishing pattern is more complex than a single property. Also the most complex solution to code since additional data needs to be conveyed to the autocomplete.

Thank you for these @frosterus ! We discussed the proposed UI alternatives and preferred the second one. However we would like to see that option in a three different modified versions:

1. the distinguisher (e.g. oppiarvo) below the concept label
2. the distinguisher(e.g. oppiarvo) below the concept label with corresponding icon (as in option 3)
3. the distinguisher and the concept type (and other possible extra information) all below the concept label in the left side
   Would it be possible for you to draw examples of these suggestions?

Also we would like to see this implementation as a step towards a solution in which all the extra information to the label in the autocomplete list (including concept type) would be configurable and included or left out all together.

[frosterus]

The next batch of UI mockups.

Here the distinguisher and the concept type are both below the label (note that skos:Concept is supposed to show the label as it does now):

Here the distinguisher is below and the concept type on the right:

In theory the icons could also be used next to the distinguisher but in that case I feel that the icons need to be featured more prominently in Finto in general as they currently are shown very rarely to the user and probably the average user doesn't associate them with the properties they represent.

[frosterus]

After review, the suggestion was to go with the latter approach but move the type indication to the top line and force it there (so if the label does not fit, it will go to two lines but the type indication is always on the top line).

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
1 Security Hotspot
8 Code Smells

No Coverage information
0.0% Duplication

[kouralex]

I have now updated this PR based on #1380 (comment). I had a conversation with @jarmosaarikko about how long labels should be wrapped altogether with the type information (which was not detailed in the aforementioned comment) and we concluded that it would be better to not wrap the concept label element in case it does not fully fit with the type information. See the image below.

Also, it was discussed that would be better to break at word boundaries (determined by the user's browser), however, there is a caveat: this does not fully work in case there are long lists of single-word types (or distinguishers) in either .concept-type or .concept-distinguisher as they are separated only by a comma that does not introduce a more preferable word boundary. This can be seen in the second image (below) where I used KOKO vocabulary as an example (as that has lots of type information). This is not different from how it is currently working (and compared to Finto's installation, on my installation the types are even longer than usual as they are shown by their full IRIs instead of labels due to caching issues), this was just an observation. Another separator might yield a better outcome, e.g., introduction of a <wbr> tag after each comma.

As a last note: in order to force type on the first row I switched the type-including element before the label element (which I wrapped in a wrapper element). This may be semantically wrong (WCAG-wise), however, proper fix would most likely require a flexbox-reaching approach (via order property) that I decided not to go for as there is, most likely, a complete UI overhaul incoming (see https://github.com/NatLibFi/Skosmos/wiki/Skosmos-3.0-Draft-Roadmap). Additionally, I have a feeling that the current implementation of autocompletetion is neither WCAG-compatible.

[joelit]

Great to see this going forward! I tested this thoroughly and managed to break this in different ways. I'm mainly concerned about the way the new configuration property is set as "prefix:uri" strings. This is vulnerable to misinterpretation in cases like "!skos:member" which results in this:

More worryingly, a mistyped URI-string (for example in a case where the prefix was not recognized) results in no results for the drop-down menu:

I'd really really like to see the configuration done as actual URI-references instead of strings to make sure the namespace prefix is interpreted correctly, and for more robustness in handling the new configuration property.

Also, as a very minor thing, the distinguishers are listed with a comma but without spaces:

[kouralex]

I agree that the prefix setting is a bit problematic (which is why it is stated in the OP under the section Known problems or uncertainties in this PR). We could, of course, instruct the configuration setter to only use IRIREFs.

The issue you had with your example !skos:prefLabel is due to ! being different from ^. I have presumed the configuration setter to have some knowledge of SPARQL in case they are interested in using this particular configuration property. Why there are no results in case of unknown prefix is due to SPARQL query failing because of this. I agree that this might be a problem, hence I propose one to use only IRIREFs via direct instructions. If this may not do, I will have to update this PR upon your request.

And yes, distinguishers are listed with a comma but without spaces because that is the way they are now implemented in the autocomplete in case there are multiple types. This can be changed but I would suggest to change it in both places, simultaneously.

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
1 Security Hotspot
7 Code Smells

No Coverage information
0.0% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

[osma]

I have the same concerns as @joelit - it seems like a very generic configuration mechanism, but the configuration syntax is ad-hoc and potentially fragile.

Nevertheless there is a clear need for this, and it doesn't seem to break anything we currently have in Skosmos or Finto, so I think this can be merged for Skosmos 2.x. For Skosmos 3, where the autocomplete component hasn't yet been implemented, I think we need to have a new discussion about the requirements. I've understood that the data model for the Metadata Vocabulary is probably going to change as well in the near future. I think that for Skosmos 3 it would be better to implement an autocomplete component with support for a relatively short selection of distinguishers (e.g. rdf:type, skos:inScheme, inverse skos:member) that can be enabled or disabled per vocabulary as needed, instead of attempting a more generic mechanism which has lots of pitfalls.

I will merge this now, so we can release it as part of the upcoming Skosmos 2.17 release.