Skip to content

Conversation

@uglyeoin
Copy link
Contributor

@uglyeoin uglyeoin commented Apr 20, 2016

Pull Request for Issue #10011 .

Summary of Changes

The class item-featured has been added to both components and modules that contain a featured item. Featured items include articles and contacts.

Testing Instructions

Install Joomla! & Patch Tester

Install Joomla! & Patch tester. Here's a video with some instructions. https://youtu.be/4OWgusZgIfk
Download here: https://docs.joomla.org/Component_Patchtester_for_Testers

The pull ID for this issue is 10011

The title is Update Joomla! content and contacts to include an item-featured class if an item is featured.

Test the issues

You will need to create (at least) 2 articles (if using sample data you can use these items). One should be a featured article, one should not be a featured article. You will need to ensure that you show both of these in your test. The easiest way to do this in a blog view is to ensure they are in the same category and the category is selected. The same is true for other views and modules.

You will need to create (at least) 2 contacts. One should be a featured contact one should not be a featured contact. You will need to ensure both of these are showing on the page you are viewing.

You will need to create menu items for the category views listed below. So for example you will need to create a menu for category blog view, article archived view, contact all contacts view.. etc. If any views are not mentioned but include an article or a contact, you should add these to your testing process and report back if featured items do not show the class item-featured.

The below list outlines the modules and components that have been changed.

Components

You should create a menu item for each of the following.

com_content - category blog view
com_content - category list view
com_content - archive view
com_content - featured articles view

com_contact - category view
com_contact - featured view

Modules

You should create and publish all of these modules. They are all part of the Joomla! core. Simply go to module manager, click on new and find the correct module. Publish it to the page you are looking at (or all pages).

mod_article_archive
mod_articles_category
mod_articles_latest
mod_articles_news - horizontal view
mod_articles_news - vertical view
mod_articles_most_popular
mod_related_items

Once you have done this, you should navigate to the menu item you have created, for example you should have created a menu item that links to category blog and contains the articles you created. Visiting the blog page should show you your articles, you should have selected one to be a featured article.

You will not notice any immediate difference, you will have to inspect the code to see if the class "item-featured" shows. You will need to repeat this step for all of the different content views.

The same is true for the modules, again, you will need to ensure they are showing on your page, and inspect the element to see if the class item-featured shows.

Finally, you will need to repeat these steps for the contact pages. The contacts that are featured should be called item-featured.

The featured contacts/content should all have item-featured as a class. Any items that you have NOT set to be featured, should not include this class.

This adds the class "featured" to the article when in category Blog view so you can style featured articles differently to non-featured articles.
<div class="items-leading clearfix">
<?php foreach ($this->lead_items as &$item) : ?>
<div class="leading-<?php echo $leadingcount; ?><?php echo $item->state == 0 ? ' system-unpublished' : null; ?>"
<div class="leading-<?php echo $leadingcount; ?><?php echo $item->state == 0 ? ' system-unpublished' : null; ?> <?php echo ($item->featured) ? 'featured' : ''; ?>"
Copy link
Member

@richard67 richard67 Apr 20, 2016

Choose a reason for hiding this comment

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

You should change this to

<div class="leading-<?php echo $leadingcount; ?><?php echo $item->state == 0 ? ' system-unpublished' : null; ?><?php echo ($item->featured) ? ' featured' : ''; ?>"
i.e. remove the space before "featured) ? 'featured' : ''; ?>" and add a space to the beginning of 'featured'.

otherwise you will have 1 space at the end of the class string if the article is not featured.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I'm new around here, good tip. I thought about whether there was an advantage to doing it that way but I couldn't work out whether there was or not. Thanks for explaining.

@richard67
Copy link
Member

You should change

<div class="leading-<?php echo $leadingcount; ?><?php echo $item->state == 0 ? ' system-unpublished' : null; ?> <?php echo ($item->featured) ? 'featured' : ''; ?>"

to

<div class="leading-<?php echo $leadingcount; ?><?php echo $item->state == 0 ? ' system-unpublished' : null; ?><?php echo ($item->featured) ? ' featured' : ''; ?>"

i.e. remove the space before "featured) ? 'featured' : ''; ?>" and add a space to the beginning of 'featured'.

Otherwise, if you don't change that, you will have 1 space at the end of the class attribute if the article is not featured.

@brianteeman brianteeman added this to the Joomla! 3.6.0 milestone Apr 20, 2016
@brianteeman
Copy link
Contributor

Personally I am not in favour of this as if you already have some styling in the template for the class featured your content display will be changed when you update joomla without your knowledge or control. To me this is the role of the template and a template override.

But if this is going to be accepted then you will need to add this to the beez template as well.


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

@uglyeoin
Copy link
Contributor Author

uglyeoin commented Apr 20, 2016

Would changing the class name to something like art-featured be a solution? The problem being without solving the issue in the core, you're really only solving it for people who use the default template.

Corrections as per comments
@brianteeman
Copy link
Contributor

Not using an existing classname would solve the first part of my comment

On 20 April 2016 at 19:03, uglyeoin [email protected] wrote:

Would changing the class name to something like j-art-featured be a
solution?


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#10011 (comment)

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@uglyeoin
Copy link
Contributor Author

I have updated Beez as per the later part of your comment. See here: #10014 My apologies if I should have done it all as one. I've used the class .art-featured on both.

Changed the class from art-featured to featured-article as it was more descriptive
@brianteeman
Copy link
Contributor

Yes you should have done it as one (and at least given correct test
instructions in the other one)

I have explained how you should add those changes to this pull request there

On 20 April 2016 at 19:12, uglyeoin [email protected] wrote:

I have updated Beez as per the later part of your comment. See here:
#10014 #10014 My apologies if
I should have done it all as one. I've used the class .art-featured on both.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#10011 (comment)

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@rdeutz
Copy link
Contributor

rdeutz commented Apr 20, 2016

For items in featured it isn't needed and I have my doubts that this is needed at all.

Added article-featured class to blog items if they are featured allowing for different styling for featured articles.
@uglyeoin
Copy link
Contributor Author

Ah ha I achieved the goal of merging them. Thanks for your help @brianteeman & @rdeutz .

@rdeutz can you explain your comment as why it isn't needed? My aim is to be able to style featured articles differently to normal ones. Is that already possible somehow?

@rdeutz
Copy link
Contributor

rdeutz commented Apr 21, 2016

@uglyeoin for leading items you could use .items-leading div {} and you can do the same, to style an item after the leading items you need your change but as I said I have my doubts that this needed at all. I never had such a requirement and you can do this easily with an overwrite for your template. At this state I don't think that is something we should put into the core.

@Bodge-IT
Copy link
Contributor

Bodge-IT commented Apr 21, 2016

I think if you have a feature called Featured in the core, you should be able to accurately select it for CSS styling without having to override the system. Sounds like a half baked feature to me.

@brianteeman
Copy link
Contributor

The object of the Featured feature is to be able to select articles from
multiple categories and display them on a single page with the featured
articles menu type. It is not to be able to format an article wherever it
appears. This is perhaps one of the oldest features of joomla that has been
in existence for over ten years. As stated before if you want to apply
special formatting to a featured article in a regular category blog view
then you can do this with a template override.

On 21 April 2016 at 12:03, Gary Barclay [email protected] wrote:

I think if you have a feature called Featured in the core, you should be
able to accurately select without having to override the system. Sounds
like a half backed feature to me.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10011 (comment)

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@uglyeoin
Copy link
Contributor Author

You can do an override, yes. But what is the point in a featured article if it has no visual difference from any other article. The very definition of featured is "have as a prominent attribute or aspect".

A featured article may be a leading item or it may not be a leading item. Styling all of the leading items with .leading-item div is not selecting featured items.

In the featured menu item it is easy to select all articles. In a mixed situation like blog. There is no way to achieve this at present.

Square Balloon

On 21 Apr 2016, at 12:07, Brian Teeman <[email protected]mailto:[email protected]> wrote:

The object of the Featured feature is to be able to select articles from
multiple categories and display them on a single page with the featured
articles menu type. It is not to be able to format an article wherever it
appears. This is perhaps one of the oldest features of joomla that has been
in existence for over ten years. As stated before if you want to apply
special formatting to a featured article in a regular category blog view
then you can do this with a template override.

On 21 April 2016 at 12:03, Gary Barclay <[email protected]mailto:[email protected]> wrote:

I think if you have a feature called Featured in the core, you should be
able to accurately select without having to override the system. Sounds
like a half backed feature to me.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10011 (comment)

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHubhttps://github.com//pull/10011#issuecomment-212860589

@brianteeman
Copy link
Contributor

I have already explained the purpose of the featured article. Not repeating
myself again.

The great thing abut joomla is that if you dont like a core behaviour then
you can override it

On 21 April 2016 at 12:20, uglyeoin [email protected] wrote:

You can do an override, yes. But what is the point in a featured article
if it has no visual difference from any other article. The very definition
of featured is "have as a prominent attribute or aspect".

A featured article may be a leading item or it may not be a leading item.
Styling all of the leading items with .leading-item div is not selecting
featured items.

In the featured menu item it is easy to select all articles. In a mixed
situation like blog. There is no way to achieve this at present.

Square Balloon

On 21 Apr 2016, at 12:07, Brian Teeman <[email protected]<mailto:
[email protected]>> wrote:

The object of the Featured feature is to be able to select articles from
multiple categories and display them on a single page with the featured
articles menu type. It is not to be able to format an article wherever it
appears. This is perhaps one of the oldest features of joomla that has been
in existence for over ten years. As stated before if you want to apply
special formatting to a featured article in a regular category blog view
then you can do this with a template override.

On 21 April 2016 at 12:03, Gary Barclay <[email protected]<mailto:
[email protected]>> wrote:

I think if you have a feature called Featured in the core, you should be
able to accurately select without having to override the system. Sounds
like a half backed feature to me.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10011 (comment)

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub<
https://github.com/joomla/joomla-cms/pull/10011#issuecomment-212860589>


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10011 (comment)

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@Bodge-IT
Copy link
Contributor

That's certainly not what Featured means to me, maybe we should rename it to Cross Category Filter?

@brianteeman
Copy link
Contributor

After ten plus years the answer is no
On 21 Apr 2016 12:35 pm, "Gary Barclay" [email protected] wrote:

That's certainly not what Featured means to me, maybe we should rename it
to Cross Category Filter?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10011 (comment)

@richard67
Copy link
Member

"To feature" can - as far as I know as a not native English speaker but with good knowledge on that language - mean to present something at a prominent place, and that's what it does, so I am very OK with the naming as it is.

@rdeutz
Copy link
Contributor

rdeutz commented Apr 21, 2016

I have to say I mixed leading and featured a bit. I agree with Brian, the purpose of this is that you can show articles in a special view. I would say when we give articles a special class when they are featured then this should be true for all views articles are presented and not only for a blog view.

Again never had the need for it and you can do it with an overwrite, nothing for core

@uglyeoin
Copy link
Contributor Author

uglyeoin commented Apr 21, 2016

You could argue that, and I would say that makes sense. But in other views i.e. featured menu item you do not see featured/non-featured articles side by side, so it is less of an issue and could be styled via the menu.

I'm all for overrides, they are a great feature, but when an addition to the core is about 40 characters, and an override creates 4 new files, I think an override is overkill. I was surprised that there was no way to style a featured article.

I had a requirement for this feature, so I used an override. It just struck me as strange that featured articles could not be featured, unless you sorted them by featured first. Surely the fact that ordering them first is an option shows that others have thought about this issue and want a way to feature featured articles?

image

Featured articles are rendered null and void in nearly all of these ordering options. I wonder if there are any other designers out there that would agree with me?

Regardless as to whether you have found a need for this feature, others have. Myself, and these guys: http://forum.joomla.org/viewtopic.php?f=624&t=773238

@brianpeat
Copy link
Contributor

brianpeat commented Apr 21, 2016

I can actually see this being very useful. If you "feature" an article in your site, having a class around that article ANYWHERE it appears means you could do things like change the link color, or add a little star next to it. It would take a lot of work to carry that through the entire system, but I think it WOULD be useful. As far as the "purpose" of the Featured articles function, anyone new to Joomla isn't going to know that there's a 10 year old purpose for that function, they're going to wonder why they can't style a featured article somewhere else without making overrides everywhere.

Basically we can 2 ways of thinking:

  1. No, we won't think outside the box, we won't expand that feature and it will stay useful in only one view. Stop asking.
  2. Yeah, that's a good idea, it would expand the use of Featured articles into other views. This can only be a good thing!

I have to be honest, I'm really tired of seeing excuses like #1 crop up again and again when someone suggests a new feature.

@uglyeoin
Copy link
Contributor Author

@richard67 the only time it places them in a prominent position if if you sort by featured first. Any other sorting does not give them any more prominence than a standard article.

@richard67
Copy link
Member

@uglyeoin No, it places them in a prominent position in the way as @brianteeman tried to explain you, e.g. on the front page.

@uglyeoin
Copy link
Contributor Author

@richard67 Only if you have the menu item set up like that on the front page. So they are only ever featured if you either set up the menu item Featured articles, or you sort by featured articles on the blog view. Anywhere else they have no prominence, that is what this pull request solves.

@JoshJourney
Copy link

It's PR decisions like this that are the straw that break the camel's back for Joomla. 10 Years ago I was extremely proud of the achievements that Joomla was running with, even 5 years ago it was still running strong. Awesome folks like @uglyeoin come in with good innovations that support existing Joomla features only to be halted by grouchy administration. I know it's not the PR thing to say, but Joomla has been over ran by bureaucrats and is giving other CMS's the progressive software market. Joomla will still stand but not to what it could have been.

And yes this is my resignation from the Joomla community. It's been a lot of fun, met a lot of great folks, and there have been a lot of great administration in the Joomla staff as well. Had one member as a college teacher.

@uglyeoin
Copy link
Contributor Author

uglyeoin commented Feb 2, 2018

@JoshuaLewos that saddens me. I would encourage you to stay. I’d hate to think my fairly innocuous pull request has cause someone to leave the project.

@C-Lodder who is your question aimed at?

@uglyeoin
Copy link
Contributor Author

uglyeoin commented Feb 2, 2018

@JoshuaLewis (sorry on my phone) see above

@JoshJourney
Copy link

JoshJourney commented Feb 2, 2018

You made a great PR, never regret that nor should you let the community make you feel that way. Unlike other people who leave things behind, I'm a forgiving lad. ;-) I may take a peek on occasion, seek out passionate developers from the Joomla world, and come back if I see that Joomla take progression more seriously in the upcoming years.

Also note that it's been a long list of things in the making. The biggest issue for me is performance on large scale community projects even with multiple data caching/compression, reducing extensions, and doing dozens of other tricks in the book including using a good host.

@mbabker
Copy link
Contributor

mbabker commented Feb 2, 2018

Reading this again, my opinion.

  • The core workflow in Joomla for featured content is to mark content for use in a special view outside the content hierarchy, with options added to an option stuffed interface to use this special state in other scenarios

  • To me it goes counter to the core workflow to introduce DOM classes in core (used or not, and if used this would actually be more disruptive because it changes existing styles) to style some content based on publish state or certain flags in the item's config (like featured) and in effect change the workflow for this specific state/flag.

  • While there may be valid use cases for it, in my best judgment this is not just a simple change to the rendered DOM but a philosophical and B/C breaking workflow change that IMO warrants a proper discussion about whether this belongs in core or not, and to be honest I doubt there is going to be a consensus either way.

@uglyeoin
Copy link
Contributor Author

uglyeoin commented Feb 2, 2018

I understand the core workflow, maybe it could be renamed selected articles. Maybe it could be removed altogether, the same thing can be achieved with modules I believe. So one less bit of code to maintain.

The people for the pull request are mainly so because of the word “featured”. I know that renaming it in itsself requires a ridiculously long discussion, but if we can never change things we’ll never progress. Users will always find Joomla! confusing.

In the event of no concensus do we just leave the discussion bubbling along forever? Does someone make a final decision? Was that a final decision? Who needs to be involved in the discussion? I’m asking because in the future I don’t want to spend so much time on a simple pull request without knowing the correct procedure:

It should probably be documented somewhere. It’s a little unfair to expect an attempted contributor to keep their pull request in sync with the core for 2 years.

@brianteeman
Copy link
Contributor

Now that you talk about renaming the "feature" I understand your use case a little more.

Some history first. Originally this was called "frontpage" and its intention was to allow you to create a front page on your web site with selected articles from many different categories. It was later changed to "featured" as you could create a page like this anywhere on the site and not just on the "frontpage".

The object of the "feature" is to allow you to display selected articles from many different categories on the same page. It was not the objective to allow you to highlight an article within a category, on a category blog for example, This sounds like what this PR is trying to achieve. (?)

@uglyeoin
Copy link
Contributor Author

uglyeoin commented Feb 2, 2018

This PR is trying to achieve the potential of highlighting an article. Although there's definitely a use case for choosing articles from different categories on a page.

The main reason is because they are called featured, so usually if something is featured somewhere it is different from other content. Much in the same way I would be raising a pull request for intro articles if they appeared at the bottom of the page instead of as introductory articles.

The name is confusing. I've used Joomla! a long time, I remember this being the favoured way to create a front page. I no longer use this method, but others may, I don't know. But let's be honest, so many people have not been using Joomla! as long as you, and I think newcomers will find this confusing and expect it to work in the way that I have created in this pull request.

My personal opinion is that if I need articles from multiple categories on a page then there's a decent chance I will need more than one page with that set up. I'm not sure if this is possible in the core at the moment but it would be with extensions.

So keeping it as it is, I think requires a rename to stop confusion. A rework so it can work on multiple pages. Or removal if it is not a commonly used feature.

I know there was an uproar at the stats collection feature, but I think an optional plugin giving Joomla! feedback about what menu items and settings are being used would be really helpful as we would then know how much this feature is used.

If it's going to stay as featured, then I think my pull request still makes sense, it doesn't detract from the original purpose. But I think the debate has been done, so I have no idea where to go from here. Unless Michaels decision is a final one, in which case, we don't really need to discuss it at all.

@brianteeman
Copy link
Contributor

The main reason is because they are called featured, so usually if something is featured somewhere it is different from other content

Yes it is different - it can be displayed on the featured items page

My personal opinion is that if I need articles from multiple categories on a page then there's a decent chance I will need more than one page with that set up. I'm not sure if this is possible in the core at the moment but it would be with extensions.

This is already possible with the core code - no extensions required

So keeping it as it is, I think requires a rename to stop confusion.

Open to suggestions to a rename although since the name was changed from frontpage to featured in june 2009 I haven't seen any comment about the name being confusing before.

@mbabker
Copy link
Contributor

mbabker commented Feb 2, 2018

Unless Michaels decision is a final one, in which case, we don't really need to discuss it at all.

Not right now it's not. But, to me it's clear accepting this PR introduces a philosophical workflow change in how featured items in core are supported. Yes, users may already be using this feature in a way that this PR supports, but it's not really a core supported feature/workflow. No, we could not introduce styling rules to the core templates before 4.0 in support of this feature/workflow as sites using Protostar or Beez3 would have their displays altered in what is probably an unwanted manner (and even then, if we did that then it kind of forces this approach to be used at all times).

I'm not totally opposed to this FWIW. But, at a higher level, to me there is a lot more to consider than just adding some classes to the rendered DOM for exactly the reasons I pointed out.

@brianteeman
Copy link
Contributor

for reference there is a similar issue regarding archived articles

@brianteeman
Copy link
Contributor

@wilsonge @mbabker you need to make a decision on this. ITs not fair to just keep it open and keep asking for conflicts to be resolved etc if you are not going to merge it

@mbabker
Copy link
Contributor

mbabker commented Apr 10, 2018

It all depends on what core wants to define featured item workflow as. My opinion hasn't changed from before, my issue isn't the proposal itself at this point but that it changes the fundamentals of what we define featured as and how integrators should work with it; right now I lean toward declining if we stick to what featured is currently defined as but if we want to change it to match these types of use cases that exist in the ecosystem then merge.

@uglyeoin
Copy link
Contributor Author

I'm not really sure why the workflow thing is such an issue "even if unused". I just means there are multiple ways to use the same button/feature or whatever you want to call it. You can use it the old way and be unaffected. If it's just a philosophical thing, then everyone can have a different opinion already. For example some people use it for a homepage and some use it for a featured page. I don't use the featured menu item at all. So already there are three philosophies and knowing this community there are bound to be even more. Joomla! is full of people using their own workflows for things.

Can you maybe explain it in a different way so I can understand where you're coming from?

@mbabker
Copy link
Contributor

mbabker commented Apr 10, 2018

Can you maybe explain it in a different way so I can understand where you're coming from?

#10011 (comment) and #10011 (comment) cover it at a high level.

I'm of the opinion that the core code should follow some sort of specification (this is what a feature was designed for, these are the use cases that we will support as a result of it; said specification should turn into a set of automated tests to validate it). Lacking a defined specification, I fall back onto how the code is designed and what the workflow is in core (and those two comments cover what I feel is the use case for featured items and inherently what the feature specification should be). The fact that users are able to use this feature in ways that core didn't design for is pretty cool, but that doesn't necessarily mean that core has to support all of those use cases either.

Fundamentally, I do believe that accepting this pull request means that core accepts this use case as a "core supported" workflow for featured items, which is a change from the existing implementation/workflow of featured items (featured items go from "only" something which is in essence a special state used to quickly group things onto a special view to items which can also be highlighted in their own way site wide). I'm not opposed to that type of workflow or feature, like I said I think it's cool that users are able to use core features in ways they weren't designed for. That's why for me accepting this PR does introduce a philosophical change, it changes the feature specification (if one existed, we don't have those for most things in core sadly).

@infograf768
Copy link
Member

I am personnaly in favour of core supporting both cases, as both make sense.

@ghost ghost added the J3 Issue label Apr 5, 2019
@ghost ghost removed the J3 Issue label Apr 19, 2019
@ghost ghost added the J4 Re-evaluate label Jun 13, 2019
@HLeithner
Copy link
Member

To be honest I didn't read the 300 comments but as Michael wrote, this is not simply adding classes to template files.

Personal I miss use the feature attribute of the articles too but I do this in template overrides and that can be done based on the use case. I also see a small information leak adding the feature class to the html tag.

Anyway I will not merge this in to J3 series and maybe it can be considered for J4 but now I would like to ask you @uglyeoin to rebase this PR on J4 or create a new issue for discussion to add this in J4.

Thx for you time. In the meantime I close this PR.

@HLeithner HLeithner closed this Jun 29, 2019
@ghost ghost added J4 Rebase and removed J4 Re-evaluate labels Jun 30, 2019
@uglyeoin
Copy link
Contributor Author

uglyeoin commented Jul 2, 2019

@HLeithner I don't think anyone would blame you for not reading the 300 comments. It's for that same reason I'll leave this closed and not rebase to J4. I never quite got my head around @mbabker s point but hey ho, I'll leave it parked here.

@mbabker
Copy link
Contributor

mbabker commented Jul 2, 2019

TL;DR adding classes to the DOM changes how featured items are used in core. That's why I didn't agree with just blindly merging this PR 2 years ago even though it does exactly what it advertises without any real side effects in the state it's in now. And as I said in #10011 (comment) if there were a feature specification (LOL) then this pull request changes that feature specification.

Nothing stops you or anyone from applying this change in your sites. But that doesn't necessarily mean this change is right for all Joomla sites.

@uglyeoin
Copy link
Contributor Author

uglyeoin commented Jul 2, 2019

Without wanting to start a full debate again, but just to learn from your much higher knowledge than mine. It feels a bit like you're saying "best practice is to write a feature specification and then build the feature. This didn't happen in this case, and in practice there's no side effects." So it's kind of like you disagree with it from a theoretical point of view?

@mbabker
Copy link
Contributor

mbabker commented Jul 2, 2019

Since there isn't a feature spec, I fall back onto what the code currently does. And based on my interpretation of the code, the featured flag isn't intended to be used on the frontend to style content in a distinctive manner like what this PR would allow to do out-of-the-box (it comes across to me as a feature to be used for marking highlighted content to be displayed on a special view, not as a feature that should be used for marking highlighted content site-wide). So that's why I believe this PR changes the definition of a featured item.

It may very well be there are a number of site owners who do want featured items to behave in the "highlight this content wherever it is displayed on the site" way, clearly that's possible now because all this PR is doing is adding classes to layouts (it's not altering any queries or other PHP code). So as long as someone is maintaining their site and using featured items in this way consistently, then it's no problem to use featured items on your own sites in this way. But, I don't think that the core definition of featured items needs to change as a result. We use the featured flag on various joomla.org sites to ensure key recent articles are highlighted and generally have the homepages set up as a featured item view, this gives us full control over what posts are visible on the homepage and what posts are only visible in their respective category feeds, with this PR in core we would have to be cautious about any CSS changes leaking into the template and changing the display of these items site-wide.

@uglyeoin
Copy link
Contributor Author

uglyeoin commented Jul 2, 2019

Thanks @mbabker

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.