[#33487] Basic JMicrodata implementation in com_content (single article view), also JMicrodata implementation in plugin Content - Vote#3330
[#33487] Basic JMicrodata implementation in com_content (single article view), also JMicrodata implementation in plugin Content - Vote#3330alexprut wants to merge 5 commits intojoomla:3.3-devfrom alexprut:microdata-implementation-com_content
Conversation
libraries/joomla/factory.php
Outdated
There was a problem hiding this comment.
Can we mark this as (CMS) 3.3 rather than 13.1 please?
|
I agree with @phproberto at #3252 (comment) about adding more parameters. Could always rendering the microdata ever harm a site? |
|
Agreed about the on/off. I guess we need the microdata type param though?? i'm not too clued up about it |
|
A page can be one of a few different types. You'd need a way to define this type, especially as the different types can have different metadata. |
|
@wilsonge @betweenbrain |
There is no platform anymore. So 3.3 would be correct. |
|
Regarding: Administration (backend panel) → Content → Article Manager → Edit (edit any article) → Options (tab) → Show Voting (dropdown list) → select Show Did you finally decided not to move it to "metadata" tab? |
|
@javigomez Do you mean the Publishing (tab)?
the dropdown is not part of the implementation, it was allready there and used to show the content rating, but as I implemented the JMicrodata library in that plugin, it must be enabled for testing |
|
Tested .. All good ! |
|
You set the default value for the microdata param to be enabled and the type to "article" in the XML definitions. But when you retrieve the parameter value you don't set any default values. The result is that microdatas aren't enabled till the user opens and saves the params, and then it magically will be enabled without the user doing something. The same is true for the microdata type. As long as the article isn't saved, the type will be "thing". As soon as the article is saved again, it will take the default "article" type. The default values used in the XML definition and the ones used in the code ( A thing to consider is to add the microdata type param to the com_content options as well and add a default "Use Global" to the article param. I can see it getting tedious to change the type for each article ;-) |
|
i kinda agree that i think we dont need to have this as an option at all. atleast not on a per article basis ? |
libraries/joomla/factory.php
Outdated
There was a problem hiding this comment.
Does it make sense to have this public? We have the getMicrodata() method to retrieve this property. I don't think extensions should be able to change that property. If we want to allow them changing it, we should provide an API like setMicrodata() which will set the property then.
I would make it private.
The other already existing properties are probably public for BC reasons
There was a problem hiding this comment.
@Bakual I agree, but if you take a look at that class, you will see that all the properties are public, don't know why.
There was a problem hiding this comment.
I'm just guessing but I'd say it's to keep backward compatibility with existing code when the scope was added 5 years ago.
However since now introduce a new property, it can be private without issues. We can always make it protected or public later if we find it needed, but we can't change a public property to private later. So I'd better start restrictive 😄
@Bakual Yes, that's right, this is because I've not updated the default SQL files used to install the CMS.
I completely agree. But I sincerely can't understand why the Article Global Configurations overrides the Article Configurations? (if I disable an option from the Article Options and that same option is enabled in Article Global Configuration, that option is enabled when the article is displayed) |
That's not how it is supposed to work. The article options should override the global ones. It gets a bit tricky if you take into account menu item params. There a menu item will override article if it's a single menu item pointing to that specific article. Otherwise article will override menu item which overrides component settings. If something doesn't behave like this, it's a bug.
Would maybe make sense as well. I don't think someone will want to disable microdata for only one menu item, but I could see someone having for example a menu item "blog" and one for "news" where the microdata type would be different. |
SQL files would only work for new installations, not for updating users. If you want to enable microdata anyway by default, then you need to do the proper default values when setting the config (what you already did in the XML) and also when retrieving the value (which you didn't do). Those two place need to match in all cases. Even if you set something using SQL files, they still need to match. Also it looks you the PR is messed up currently :-( |
…/disable microdata semantics
…le microdata semantics
plugins/content/vote/vote.php
Outdated
There was a problem hiding this comment.
Is this line supposed to be $microdata = JFactory::getMicrodata($params->get('microdata'))->setType(json_decode($row->attribs['microdata_type'], true));?
There was a problem hiding this comment.
@betweenbrain No, the way it is should work.
I saw what you've reported on JTracker, I don't get any warning, anyway, I've made some fixes, can you please test again. thanks
|
@Bakual I've added the default values when retrieving a param. Can you please check again?
Working on that. |
Fixed. |
|
Hi Alexandru, |
|
Enabling/Disabling microdatas in global configuration doesn't seem to have any effect at all. That would probably need a "Use Global" in the component settings as well, and then fall back to global config if "Use Global" is set. Or just remove the global config setting. After playing around with it a bit more, I think the library implementation is to complicate and inflexible to use compared to just implementing hardcoded microdata. Have a look at PR #3358 to see how easy it actually would be to implement the microdata attributes directly. |
There was a problem hiding this comment.
otice: Undefined index: microdata_type in /.../plugins/content/vote/vote.php on line 57 if the article opions aren't saved. That's because you don't use JRegistry and no default value here to read the param.
|
Agree with bakul in enabling micro data in global context. |
You're right 👍 , for the parts that are implemented for the moment the Global Configuration only affects the Content - Vote plugin, I'm agree about adding a Use Global in the Article Global Configuration, but: how do I maintain a consistent UI, all the options in that interface are (Yes/No) or (Show/Hide), if I'll add a Use Global option it will result in (Use Global/Yes/No) and break the consistency, or should I use a dropdown list? |
We do that inconsistent currently even within com_content. If you look at the article options, we use lists. If you look at a menu item for a single article, we use radio buttons. |
|
Yeah a list would do well, The usability in the article options is not very promising, it brings flexibility but in a way its really hard to find an option at times. What about using another field for "Micro Data" just a suggestion :). But com_content options definitely need some re factoring to do in a way web master could easily find an option. |
|
A couple of issues/thoughts on this PR:
I hope that's helpful, will try to help with this if I can! |
|
@PAlexcom one of my concerns about this is the amount of PHP code it introduces into the view files. To help mitigate this issue, would it be possible to move the microdata processing into an earlier step in the execution cycle so that it modifies the item properties before this stage? That is |
|
I wouldn't do it further up the stack. For example the title may also be used in other places like a tooltip where you don't want to have HTML tags around it. I would rather change the API to be more simpler. For example why do we need that: |
|
Thanks @Bakual, that's a good point about modifying the existing properties. @AmyStephen's comment at #3358 (comment) got me thinking and I fully agree that we need to prevent "reaching out of the view". Beyond complicating the code in the views, instantiating a class could have performance implementations. With the very excellent idea of implementing this as a plugin, I'd suggest that we add a To optimize backend performance, we could do this onBeforeSave to update each item, but we'd to also need to fire a one-time event to index items. Maybe that could be done in conjunction to enabling microdata globally, to update all, and them on a category basis to just update that category. Thoughts? |
If the Type (Article, Thing ...) not changed in the meantime, yes :) |
Well, because for example: you can set your Property at line 1, and then you set your Fallbacks or Type somewhere else, so until display() isn't called it will not be rendered. It's the same approach when you build a SQL query. What do you think? |
|
Nice work Alex but I have few questions. So far we have introduced this in com_content which mostly behaves as a blog article. Or see this for a book In Joomla , we are missing half of those tags that we see in recipe/book types, Template dev here , hope you understand. Thank you! |
|
Tested with default install of 3.3-dev branch (as of 6pm GMT today) + patch Functions as expected and described. One thing I did notice is the author microdata is always the "created by" entry (or their alias) even when an Author is explicitly named in the Publishing tab. @danyj <footer class="hidden" >
<p>Publisher: <span itemprop="publisher">KISS Web Design</span></p>
<p>Audience: <span itemprop="audience">Joomla Testers for PR#3330</span></p>
</footer>Although these additions would be visible in the 'articlebody' property when viewing the extracted structured data. This would not be the case with an override. The output from Google's Structured Data Testing tool looks like this A template override would be the best way to add any properties that are not included. Either way though there is no impact on SEO for not using some of the available properties, just look at how many are listed for the Blog schema http://schema.org/Blog and nobody uses all of them. Perhaps an example override file (with lots of comments about what, why and how) could be produced for template devs to use as a reference - and then added to the documentation. Not sure if I'll have time to do this myself, hence leaving the note here ;-) |
I would definitely consider that a bug |
Well as you know by default the com_content was designed to be used and output Articles/Blog, but the end user can use it to write Recipes or everything else (there are 558 different types). The only valid information that we know and are in common is http://prntscr.com/38fsrg
We don't and can't know what type of semantics/context the author uses in the content (article body), so that type of information needs to be added by the author (planned for the future: to add semantics from the content editor).
As I know it should not harm the site SEO, but I know for sure that if the semantics are used in the wrong way for sure will harm the website, the library takes care to check and display valid semantics.
Yes and No, in the current com_content view override yes, some
We need your help :) The current implementation is a bit complex, I've created a new post in order to get some feedback and simplify the current implementation https://groups.google.com/forum/#!topic/joomla-dev-general/s0SR7hyfW_I |
|
Thnx Alex, I saw your post on JGD, According to the answer , Google might cut you off completely if microdata is "manipulated" I am finishing our framework and yes we do use our own overrides. Our framework works on 2.5.x and 3.x still waiting to see what you make out of it so that I can fit it in 2.5.x or just display the microdata on 3.x. |
|
@dongilbert @KISS-Web-Design
I would not consider it a JMicrodata library bug |
|
If we are outputting the wrong author then its a bug. |
|
@PAlexcom - agree with @brianteeman - here is an example of how to determine |
|
@PAlexcom see mys post here please https://groups.google.com/forum/#!topic/joomla-dev-general/s0SR7hyfW_I |
|
I'm closing this PR as it is against an outdated branch. |
A basic implementation of JMicrodata library in the CMS , implemented the library in com_content (single article view) and in plugin Content - Vote.
Testing instructions (todo list)
Backend
you should find a toggle button to enable/disable globally microdata semantics
Frontend
You should play around to see if everything works fine, if you have problems or difficulties in testing let me know, I will help you.
Small 'problems/questions'
If this implementation works fine and the feedback is OK, I start with other PRs in order to finish the implementation of JMicrodata in com_content and other components/modules/plugins.
Thanks.
JTracker http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33487&start=0