? Success

User tests: Successful: Unsuccessful:

avatar photodude
photodude
18 Jan 2016

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/

avatar photodude photodude - open - 18 Jan 2016
avatar photodude photodude - change - 18 Jan 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 18 Jan 2016
Labels Added: ?
avatar photodude photodude - change - 18 Jan 2016
Title
Update to use JMicrodata library rather than hard coded values
Update latest articles module to use JMicrodata library rather than hard coded values
avatar photodude photodude - change - 19 Jan 2016
Title
Update latest articles module to use JMicrodata library rather than hard coded values
Update to use JMicrodata library rather than hard coded values
avatar brianteeman brianteeman - change - 20 Jan 2016
Category Code style
avatar wilsonge
wilsonge - comment - 20 Jan 2016

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 :)

avatar photodude
photodude - comment - 20 Jan 2016

Thanks @wilsonge I tried to find the justification, or PR, for hardcoding the microdata, but I didn't come accross #3358
I'll look over that discussion before continuing.

avatar bertmert
bertmert - comment - 20 Jan 2016

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

avatar ggppdk
ggppdk - comment - 21 Jan 2016

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'); . '>';
avatar Bakual
Bakual - comment - 21 Jan 2016

There was a PR to include a plugin which had a quite simple markup to then generate proper microdata: #4118

avatar brianteeman
brianteeman - comment - 25 Jan 2016

Setting to needs review so the cms maintainers can make a decision based on comment above by @wilsonge


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8933.

avatar brianteeman brianteeman - change - 25 Jan 2016
Status Pending Needs Review
avatar sandstorm871
sandstorm871 - comment - 3 Feb 2016

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.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8933.

avatar photodude
photodude - comment - 5 Feb 2016

@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...

avatar photodude
photodude - comment - 5 Feb 2016

@ggppdk I agree The JMicrodata api could should be made more readable

avatar sandstorm871
sandstorm871 - comment - 5 Feb 2016

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

avatar photodude
photodude - comment - 5 Feb 2016

@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).

avatar photodude
photodude - comment - 5 Feb 2016

@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.

avatar wilsonge
wilsonge - comment - 7 May 2016

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

avatar wilsonge wilsonge - close - 7 May 2016
avatar wilsonge wilsonge - change - 7 May 2016
Status Needs Review Closed
Closed_Date 0000-00-00 00:00:00 2016-05-07 11:04:51
Closed_By wilsonge

Add a Comment

Login with GitHub to post a comment