User tests: Successful: Unsuccessful:
We have the JMicrodata library, lets actually use it.
Testing
1. Add patch
https://docs.joomla.org/Testing_Joomla!_patches
2. modify the template to remove any overrides for com_content
or mod_articles_latest
(you can't test if you have an override, since you would just be testing the override)
3. Add a latest articles module to site display
4. add some articles, arcived articles and blog layout menu types for displaying on the site side
5. Check that microdata values are still applied using a microdata validation tool
https://developers.google.com/structured-data/testing-tool/
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Title |
|
Title |
|
Category | ⇒ | Code style |
Using library would be a nice first step to get rid of microdatas in HTML at all if there would be a simple single switch somewhere to suppress output.
For guys that use JSON-LD (more flexible than microdata if Google changes structured datas rules (again and again and again)). Now you have to create several overrides or have to remove microdata by plugin. Not possible to show them only robots. And so on...
Just saying that
The JMicrodata api could be made more readable
e.g. display(...) is commonly called without parameters
i suggest that these are added:
public function __toString()
{
return $this->display();
}
public function itemprop($property)
return $this->property($property);
}
so that
'<span' . echo $microdata->property('name')->display(); . '>';
it can be made (which is more readable and searchable):
'<span' . echo $microdata->itemprop('name'); . '>';
Setting to needs review so the cms maintainers can make a decision based on comment above by @wilsonge
Status | Pending | ⇒ | Needs Review |
Tested the patch anyway (just noticed its set to needs review) & after patch, each article title now has a ">" at the start of it so, I guess the patch needs cleaning up before testing further.
@sandstorm871 Can you give additional details on where your seeing titles with additional ">" at the start?
I have reviewed a site with the Joomla sample data and this patch and I'm unable to comfirm the reported issue with this patch. (tested the Joomla defult templates and a custom template with HTML overrides removed)
Maybe there is a template or plugin specific issue causing the addional ">" at the start of the title...
http://www.mynewsite.me.uk/ace
With patch there is an additional > at the start of the title.
If I revert patch the > has gone
@sandstorm871 Thanks. I'm not sure what's happening but, I now can see it's with the mod_articles_latest changes.
I'll see if I can pin down what is at the root of that extra caret (or pull those changes from the PR).
@sandstorm871 mod_articles_latest changes was the first item I changed. Looks like I had used a content('')->
in the setup for the JMicrodata which resulted in an additional span being added in the code resulting in a tag ending mismatch.
There are still plenty of nuances to the JMicrodata API I don't yet understand.
In anycase the reported extra caret > before mod_articles_lates article titles should be resolved
remove the current patch, update the patch tester list, and reapply this updated patch and it should work as expected.
Myself, Chris, Roland and Viktor have just discussed this and I think our decision is to stick with hardcoding this for the same reasons we chose not to use it in the first place. I'm going to close this PR and the other module ones for this reason
Status | Needs Review | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-05-07 11:04:51 |
Closed_By | ⇒ | wilsonge |
I mean this was a deliberate decision to hardcore (see discussion at #3358). Whilst I'm open to reviewing those kinds of decisions I don't think much (if anything) has changed since then. But willing to hear arguments otherwise :)