Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hardcoded Microdata for com_content #3358

Merged
merged 6 commits into from Apr 2, 2014
Merged

Conversation

Bakual
Copy link
Contributor

@Bakual Bakual commented Mar 23, 2014

This PR introduces Microdata tags to com_content in a hardcoded way.
This is based on some feedback that we don't need more parameters.

The PR is based on PR #3252 and the newer PR #3330. But unlike those two it doesn't use the Microdata library and instead just hardcodes the tags into the layouts.

Advantage

  • Easy to understand
  • Easy to change for endusers and designers who like to override a layout

Disadvantages

  • You can't enable/disable Microdata tags globally or per article. You can of course use layout overrides and alternate layouts to achieve the same. It's also to note that there isn't really a reason to turn the Microdata off.
  • You can't select a different Microdata type for an article using a parameter. It's hardcoded to be "Article" for single articles and "Blog" & "BlogPosting" for the blog layout in the category and featured view. You can of course use layout overrides and alternate layouts here as well to achieve the same, but you need to make sure yourself that the properties are valid for the selected type.

Testing

Test the different views of com_content and check the microdata output using the Google richsnippet test tool: http://www.google.com/webmasters/tools/richsnippets
If the vote plugin is enabled, the search result should show the voting as well.
The latest articles module contains also some Microdata tags with this change.

@Bakual
Copy link
Contributor Author

Bakual commented Mar 23, 2014

@betweenbrain
Copy link
Contributor

@Bakual when you say

You can't select a different Microdata type for an article using a parameter.

Is that option supported with another PR somewhere? I don't see that ability with #3252 or #3330. Just makin sure that I didn't miss something 😄

<div class="page-header">
<h2>
<h2 itemprop="name">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at some of the examples at http://developers.whatwg.org/microdata.html#microdata, I'm inclinded to think that, in this case, itemprop="name" should be implemented as <span itemprop="name"> within the anchor (as much as I hate to add more spans to the page).

I'm also wondering if it should be itemprop="headline" instead. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at some of the examples at http://developers.whatwg.org/microdata.html#microdata, I'm inclinded to think that, in this case, itemprop="name" should be implemented as within the anchor (as much as I hate to add more spans to the page).

I understood it that you can add itemprops to any element you want. The only special tag is the <a> element because the itemprop targets the href attribute there instead of the content. Looking at the validator the <h2> currently behaves correctly. And I hate adding unneeded spans as well 😄
But if there is an issue with that, we sure can change it.

I'm also wondering if it should be itemprop="headline" instead. Thoughts?

headline may work as well. Looking at their example at http://schema.org/Article they use name however for the title. Maybe headline is meant to be more something like our intro text? But I'm not native english and can't say what "headline" exactly means.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. As the saying goes, leave well enough alone 😄

I do think this underscores the need to keep microdata as flexible as possible.

@drmmr763
Copy link
Contributor

So I think this looks great. I will try to test it later. My biggest question is this: is there a reason that the Microdata API can't be used here? To me this is what it's "for", but I know there is some current discussion on the implementation. I'm just trying to further understand what we have in that library and what it can actually be used for, and why it isn't a good fit for this.

@betweenbrain
Copy link
Contributor

@drmmr763 my personal take on it is that the Microdata API, in this case, adds more complication than benefit. For example, to render itemprop="name" you need to add $microdata->content($this->escape($this->item->title))->property('name')->display(); to your view. This makes working with the views more difficult than necessary with little benefit from what I can tell.

Hardcoding a good, baseline microdata implementation eliminates the need to add more parameters to the backend of Joomla while still allowing granular implementation by the end user. I also don't see users wanting to change the schema they use on a regular basis as well.

@Bakual
Copy link
Contributor Author

Bakual commented Mar 24, 2014

Is that option supported with another PR somewhere? I don't see that ability with #3252 or #3330. Just makin sure that I didn't miss something 😄

PR 3330 allows to change the itemtype on a per article basis. See https://github.com/joomla/joomla-cms/pull/3330/files#diff-b954a5e4248ac75dcf9086e50b845f82R427. It has the following options:

  • Article (default)
  • BlogPosting
  • MedicalScholarlyArticle
  • NewsArticle
  • Recipe
  • Review
  • TechArticle
  • Thing

However I question the need for that. Some options are subtypes of the Article one. But until you add specific properties from that subtype, there is not much value to using it.
Other types like Recipe will render properties like "articlebody" invalid. The library takes care of htat and will not show it then (I think). Again here, as long as you don't add the specifc properties, it doesn't add any value imho.

@betweenbrain
Copy link
Contributor

Thanks, I didn't see that option, only the enable/disable settings. I also question the need for that. It seems to be getting more complicated than it needs to be.

@Bakual
Copy link
Contributor Author

Bakual commented Mar 24, 2014

So I think this looks great. I will try to test it later. My biggest question is this: is there a reason that the Microdata API can't be used here? To me this is what it's "for", but I know there is some current discussion on the implementation. I'm just trying to further understand what we have in that library and what it can actually be used for, and why it isn't a good fit for this.

@drmmr763 I would put the question the other way. What is the reason to use the Microdata API? I agree that this is what the library is for, but lets have a look at the features it offers:

  • Enable/disable microdata globally. Imho there is no reason to disable it.
  • Check and fallbacks in case a property isn't supported by a type. This is of quite situational use.

My personal view is that the API is to complicate as Matt already wrote. If I need some complicate code to implement a simple attribute, then something is wrong.
The library also is quite inflexible as it for example doesn't allow to put the dates into a <time> tag like I do in this PR. At least I don't know how it would work.
I actually waited implementing microdata into my own extension because I wanted to see the implementation in core. But now that I saw it, I'm sure I will not use the library in my component.

I also agree with people saying we need to work to actually reduce the amount of parameters we have. And this is certainly something which could be achieved with layouts overrides instead of having parameters.

@alexprut
Copy link

@Bakual @betweenbrain @drmmr763

The JMicrodata library was designed with this goals in mind:

  1. Having the possibility to switch the Microdata Type dynamically with one button, without the use of other view overrides, you just change the Type (there are 558 different available types).
  2. Display validated semantics, the library takes care to display it correctly.
  3. Enable/disable the microdata semantics (if you use Joomla in your private lan, you don't need microdata)
  4. Fallbacks, you should never lose any meaningful semantic.

Example

Let's suppose that you have the following code which is part of your article, and you've selected from the article options the Article type.

<div <?php echo $microdata->displayScope();?>>
    <!-- Author of the content -->
    <span>
        Written by <?php echo $microdata->content('John Smith')->property('author')->fallback('Person', 'name')->display();?>
    </span>
    <!-- The content -->
    <?php echo $microdata->content('Here is the article text...')->property('articleBody')->display();?>
<div>

It will render:

<div itemscope itemtype='https://schema.org/Article'>
    <!-- Author of the content -->
    <span>
        Written by
        <span itemprop='author' itemscope itemtype='https://schema.org/Person'>
            <span itemprop='name'>John Smith</span>
        </span>
    </span>
    <!-- The content -->
    <span itemprop='articleBody'>Here is the article text...</span>
<div>

Instead, if you decide to change the current Type, let's say that you've selected from the article options the Thing type
It will render:

<div itemscope itemtype='https://schema.org/Thing'>
    <!-- Author of the content -->
    <span>
        Written by
        <span itemscope itemtype='https://schema.org/Person'>
            <span itemprop='name'>John Smith</span>
        </span>
    </span>
    <!-- The content -->
    Here is the article text...
<div>

As you can see John Smith fallbacks to the Person type, and there is no loss of information, even if the current Type doesn't have an author Property it will display important semantic information for the machines, the search engines know that there is a Person John Smith. And everything is valid, done fast and automatically.
Instead, if you don't need all that microdata information, you just disable that feature.
It will render:

<div>
    <!-- Author of the content -->
    <span>Written by John Smith</span>
    <!-- The content -->
    Here is the article text...
<div>

Once again everything is done by the library. You don't need 558 different view overrides, you just play with the option buttons.

The Pull Request #3330 is just a simple implementation that works, but there are other things that needs to be added, but I will not start implement elsewhere until it will be approved. I've added only a few Types (Article related) in the Mircrodata Type dropdown list, just as an example, but there are 558 different available types. I hope everything is clearer now.
If you've played with microdata semantics apparently they are easy to use, but when you want something that changes dynamically it becomes complex.
If you have a better solution, I'm here to help and improve.

@Bakual
Copy link
Contributor Author

Bakual commented Mar 25, 2014

I perfectly understand the reason for the library and what it does.
But I personally think it's unneeded as I explained already. Especially disabling microdata serves no purpose as having them enabled doesn't hurt in any case.
I can see the benefit of checking for valid microdata properties, but as already explained my personal opinion is that the implementation is way to complex for the benefit it brings.

Honestly, why would you want to changethe microdata type from Article to another type as long as you don't add specific properties for that type? With current implementation you could only loose properties, you will not gain anything until we make it even more complex than it already is.
Also what will happen if someone wants to change the Blog/BlogPosting (in a category view) to maybe ItemList/itemListElement. Then it will get a lot trickier already.

If you have a better solution, I'm here to help and improve.

I don't have a better solution than having it hardcoded. And honestly, I don't think we need a better one, and I think we never needed a complex library for that. But that's my personal opinion and people thought differently last year. I have to admit I found the idea of a library interesting as well, however now that I had to work with it, I don't like it anymore. Sorry for that.

Please keep in mind that the layout files are intended to be used and edited by template designers and users. Both groups prefer HTML/CSS code over PHP code and they likely understand Microdata semantics better than PHP if they want to change something.
We are also moving into the direction of having all HTML output generated by the layouts. The microdata library goes in part into the other direction by outputting spans and divs.

It's not meant offensive at all. I appreciate the work you did. However I think you agree that the library is a lot more complicate to use than just using hardcoded attributes.

@Fedik
Copy link
Member

Fedik commented Mar 25, 2014

my 5 cent: I think JMicrodata will perfectly fit in to UCM 😏

@RCheesley
Copy link

I do not agree with hard-coding the microdata in this way for distribution with the CMS used by millions of websites for completely different purposes.

It is important that we give the end-user the ability to properly use the semantic markup and declare what type of content is being delivered on the page, in a dynamic way which reflects the fact that often websites have semantically different content in different areas of their site - or might be specialising in a specific area and hence need to explicitly choose to use a particular type of microdata

Forcing people to use Article is not acceptable for such a diverse CMS with so many different implementations.

That being said, there is room for improvement in how we do this - for example being able to set this on a category level - but I am strongly not in favour of hard-coding the actual microdata in the way this PR proposes.

Yes, it might be simple and in fact this is what I've been doing for the past two years, manually, on virtually every site I do, but it does not mean that this is appropriate for inclusion in the CMS in this way.

When you're hard-coding you have the ability to discern what is an article, what is a news article, what is a blog, what is a recipe, and choose to use the appropriate semantic, but the average person would not.

The JMicrodata PR only includes a few itemprops actually, we can use a lot more but the aim was to keep the implementation simple to start with to allow people to become familiar with semantic markup, and then look at marking up other areas.

I hope this helps, I won't cross-post on the other hard-coding PR but my thoughts apply to both.

@brianteeman
Copy link
Contributor

That makes a lot of sense Ruth

@betweenbrain
Copy link
Contributor

Unfortunately, I don't feel that the JMicroData implementation is intuitive or easily used by those working with views on a code level and will make creating and customizing templates more difficult. This was the motivation for creating a hardcoded implementation, supported by feedback that I obtained.

@brianteeman
Copy link
Contributor

And that makes a lot of sense too Matt.
It's certainly not a simple choice to make between ease of use and correct functionality. I've not dug into microdata too much myself yet but it does "appear" to be wrong to say that Joomla has microdata but you can only do a part of it, because the options are hard coded

@RCheesley
Copy link

@betweenbrain how would it be made easier? We wrote a plugin a year or so ago which massively simplified it, basically meaning you choose the schema to use and then just surrounded the content in curlies with the itemprop - I've sent this to @PAlexcom for potential simplification.

@RCheesley
Copy link

So an example would be {microdata person name} for example - and the curlies could be nested - e.g. {microdata movie} {microdata director} {microdata person}{microdata name} James Cameron {/microdata}{/microdata}{/microdata}{microdata title}Avatar{/microdata}{/microdata}

(That's just a real basic example without any HTML/PHP included, just to give an idea, and it's probably not the best way, working from memory ;) )

Ruth

@betweenbrain
Copy link
Contributor

This is a very difficult one for me as I see value and drawbacks in both approaches. I'm also looking at this more holistically than just the specific implementation of microdata.

My concerns are as follows:

  1. [#33487] Basic JMicrodata implementation in com_content (single article view), also JMicrodata implementation in plugin Content - Vote #3330 adds more parameters to the backend for dynamic functionality where it isn't necessarily needed. We have more than enough parameters to drown a developer and I'm not sure if they are really needed. I asked on Twitter about this, and even @brianteeman agrees that we don't need more parameters.
  2. I agree with @AmyStephen that we need "more markup in views - less configuration of views via parameters". The issue here is that configuration of views via parameters leads to more complicated code in views, weakening the ability to easily customize the output of Joomla. Following are three screenshots of the markup of the single article view. A file that many themers regularly override.
    The current single article view
    joomla-staging-single-article
    The current single article view with hard-coded microdata
    joomla-staging-single-article-hardcded-microdata
    The current single article view using JMicroData
    joomla-staging-single-article-jmicrodata
    As you can see, they get progressively more complicated.
  3. I also recognize the need for the scheme to be able to be customized. The feedback I received on Twitter was that most people would create template overrides as necessary. In fact, alternative layouts would be a good way to implement this while keeping the view files as simple as possible.

All that being the case, I also recognize that @PAlexcom's work on JMicroData is great. I'm just concerned that we keep heading down the wrong path of adding more parameters to the backend, making it harder to use, and more PHP to views, making them harder to customize. Joomla has always been one of the easiest to theme CMSes but we seem to be losing that way.

@brianteeman
Copy link
Contributor

I'm all for less parameters, we have too many right now without adding more, but we mustnt confuse "options" to turn things on and off with "things" to select content.

At the end of the day microdata is a form of content and if I'm going to put content on my site then I want to make sure its the correct content.

My initial impression is that microdata should be selected by a generic on/off option and if switched on then you need to have all the itemprop available to you. There is no point in having it if it is wrong.

@RCheesley
Copy link

@betweenbrain so in addressing the cluttered parameters, would it be an option to only show the microdata parameters for example if it's been enabled in the global configuration?

That way you would only see the options if you're interested in using it, when you would probably want to see them.

In regards the coding complexity, I'm all for simplifying - the more needed to enter the more likely an error is to be made.

@brianteeman
Copy link
Contributor

+1

@betweenbrain
Copy link
Contributor

@RCheesley Yes, hiding the parameters when not enabled is an option. Then again, is there an argument for never using microdata? Since microdata is a way of describing your content in a very precise way, I don't see any harm in it always being used. It's not just about the parameters being visible, it's also the added code to support them. Minimizing the number of parameters needed to support microdata would be worth a look.

If it were possible to simplify the code that renders microdata, I might feel differently about rendering it dynamically. As it is now, I feel that it is too complex and not intuitive.

@RCheesley
Copy link

@betweenbrain personally I don't think there is a reason not to use semantic markup. However, there is every reason to need to configure it if it's going to be implemented in core Joomla.

As I mentioned above, Joomla is used in so many different situations that we cannot simply force the semantic explanation of 'Article' because we don't want to support the ability to properly define the content. For example, if we were to force Article, anyone using Joomla as a Blog site or recipe portal would be at a disadvantage.

With Joomla's aims being broad in terms of being a digital publishing platform, I think we should step out of our own use cases and consider the width and breadth in which Joomla is (and could be) used around the world.

That being said, I fully agree that it's too complex in the current implementation, and I have expressed this to @PAlexcom and others who have asked me about this implementation. There may well be potentially better ways to implement this, however it takes involvement of experienced developers to explore how that might be achieved.

While I have the knowledge of semantics, I do not profess to have much experience with regards to code implementation, hence why I've been speaking about microdata at events for the past couple of years to try and find people who can :)

Thank you everyone for the input in this regard, I'm sure we can find a route forward which brings together the comments regarding the importance of semantic markup and the need to keep the code complexity and parameters to a minimum.

@betweenbrain
Copy link
Contributor

@RCheesley Just to clarify my position, I completely agree with you about it ultimately needing to be as flexible as possible. I appreciate everyone's input and feedback and this has given me some ideas on how we might be able to implement it in a simpler and flexible way. Now to find the time to work it out 😉

@AmyStephen
Copy link
Contributor

Spent some time reviewing this.

tldr - I agree with the direction of this PR. This PR adds semantic content in a generic and useful way that should not require configuration. It treats the data elements (ex. title, date, URL, author) the same way for the semantic output as the visual display. Meaning: the semantic markup will be useful for those who already find the layout useful. No configuration or specific knowledge required.

A few architectural comments on Views (layouts):

  1. Views should be 'logic-free.'
  2. There is already way too much complexity in the Joomla layouts -- cloning is difficult.
  3. Article is a generic term fitting for this use. Both schema.org and WC3 specs are broad and suitable for this purpose. (Even for recipes, tho, that use case is so specialized it would almost certainly require a new component or, at minimum, a layout override.)
  4. It would seem suitable to consider a development standard prohibiting the instantiating of classes in views or "reaching out of the view" to obtain data (exception being localization). Instead of using something like $microdata->displayScope(); it would be better to add a plugin to the onPrepareContent event that appends additional element to the data object. Views should not be executing class methods, trying to rearchitect JHtml, etc., is job enough. It would be important not to add more of this approach.

Seems to me this PR achieves the goal of moving the markup forward in a way that should not require configuration by using simple, generic, useful semantic markup and in doing so avoids adding complexity. In my opinion, this is well done. The core layouts should meet the basic needs of the masses and it seems to do just that.

@Bakual
Copy link
Contributor Author

Bakual commented Mar 28, 2014

Personally, I would even go further into the direction of reducing parameters and let the user edit the layout instead. Like for example why do we need a parameter to tell if the article info is going to be before or after the article? That should be a template thing and is likely a consistent setting over the site anyway. Moving it around in the layout is easy.

Given that we now can create and edit layout files in the template manager (thanks to another GSoC project) it is actually very easy to adjust the output. You don't even need special knowledge to create the overrides.
I think this is the direction we should go. Reducing parameters and improving backend creating/editing of overrides/alternate layouts.

It's also to note that the microdata in this PR is hardcoded into the layout file. It's very easy to override and many templates have overrides for this layouts anyway. It's not like it's hardcoded into a JHtml function with no way to adjust it. So "hardcoding" may be a misleading term chosen by me.

It's also to note that if you only change the microdata type from "Article" to "Review" or "Recipe" using the library, you gain nothing but loose the now invalid attributes (only "articleBody" for now, but "articleSection" and "wordCount" could be used as well). You would have to add manually the new matching attributes like "itemReviewed" or "cookingMethod" to make it meaningful.
However if you have to edit the layout anyway, changing the type is the simplest thing to do.

We wrote a plugin a year or so ago which massively simplified it, basically meaning you choose the schema to use and then just surrounded the content in curlies with the itemprop

That sounds like an interesting approach as well. The downside is maybe what happens when the plugin is disabled. We would likely see a lot of curly braces which aren't replaced by the plugin (since it's disabled).

mbabker added a commit that referenced this pull request Apr 2, 2014
Hardcoded Microdata for com_content
@mbabker mbabker merged commit d6d3007 into joomla:3.3-dev Apr 2, 2014
@Bakual Bakual deleted the MicroDataContent branch April 2, 2014 06:21
Bakual pushed a commit to Bakual/joomla-cms that referenced this pull request May 12, 2014
Hardcoded Microdata for com_content
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants