Show tooltips (rdfs:comment or skos:definition) for custom properties #995
Conversation
Codecov Report
@@ Coverage Diff @@
## master #995 +/- ##
============================================
+ Coverage 59.12% 59.16% +0.04%
- Complexity 1548 1552 +4
============================================
Files 32 32
Lines 4335 4386 +51
============================================
+ Hits 2563 2595 +32
- Misses 1772 1791 +19
Continue to review full report at Codecov.
|
model/ConceptProperty.php
Outdated
| * @param string $prop property type eg. 'rdf:type'. | ||
| * @param string $label | ||
| */ | ||
| public function __construct($prop, $label, $super=null, $sort_by_notation=false) | ||
| public function __construct($prop, $label, $tooltip=null, $super=null, $sort_by_notation=false) |
There was a problem hiding this comment.
One could document any new parameter(s) here.
On a side note, it's also a bit risky to add a parameter in the middle of a constructor call parameter list, but in this case it does not cause trouble. It may very well be intended, though, if one thinks it is a more important one to have.
There was a problem hiding this comment.
You're right, the phpdoc declaration for the new parameter is missing.
I added the parameter in the middle because in PHP, only the last parameters can be omitted. Since there is a need to call the constructor both with and without the last two parameters, this was the only clean option. I did check all calls to the constructor.
| @@ -47,7 +50,7 @@ public function getLabel() | |||
| } | |||
|
|
|||
| // if not, see if there was a label for the property in the graph | |||
| if ($this->label) { | |||
| if ($this->label !== null) { | |||
There was a problem hiding this comment.
Just a side note: I can see why this was done, but does not - nevertheless - change the code behavior a bit. But one can argument for code quality measures, and I agree.
There was a problem hiding this comment.
I copied and pasted some of the code for the label method into description and noticed this potential problem, so I fixed it in both places. In PHP, a string containing a zero "0" evaluates as false. It's an unlikely label, I know, but still, the behavior does change a little bit here ;)
| * @return string | ||
| */ | ||
| public function getDescription() | ||
| { | ||
| $helpprop = $this->prop . "_help"; | ||
|
|
||
| return gettext($helpprop); // can't use string constant, it'd be picked up by xgettext | ||
| // see if we have a translation with the help text | ||
| $help = gettext($helpprop); |
There was a problem hiding this comment.
This will prefer Skosmos translations, but that will must likely be OK.
There was a problem hiding this comment.
Yes, that's the intent (same as for property labels)
view/concept-shared.twig
Outdated
| @@ -62,7 +62,7 @@ | |||
| {% if property.getSubPropertyOf != 'skos:hiddenLabel' %} | |||
| <div class="row{% if property.type == 'dc:isReplacedBy' %} replaced-by{% endif%}"> | |||
| <div class="property-label"> | |||
| <span class="versal{% if property.type == 'rdf:type' %}-bold{% endif %}{% if not (property.type in property.description and '_help' in property.description) %} property-click" title="{{ property.description }}"{% else %}"{% endif %}>{{ property.label|upper }}</span> | |||
| <span class="versal{% if property.type == 'rdf:type' %}-bold{% endif %}{% if property.description %} property-click" title="{{ property.description }}"{% else %}"{% endif %}>{{ property.label|upper }}</span> | |||
There was a problem hiding this comment.
This could only have an if part.
|
Kudos, SonarCloud Quality Gate passed!
|
The tooltip content is read from
rdfs:commentorskos:definitionvalues of the property URI.Example of Finnish Corporate Names showing RDA property definition as a tooltip:
Fixes #824