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

[5.2] Content module #43610

Closed
wants to merge 18 commits into from
Closed

Conversation

brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Jun 3, 2024

Pull Request for Issue #43404 .

Summary of Changes

This single module is intended to do everything that all the other articles modules can do but in a single unified module.

With this module you can create modules either as a list of title only or with all the different parts of an articles.

In addition you can group and filter content by a variety of criteria

Testing Instructions

apply the pr and discover install the module.
You can now create as many content modules as you need in order to test all the different options

Note

There may be some legacy style code which should be replaced (pr welcome)

The default layout is intentionally crude but it can be styled if requested (pr welcome)

This is not a direct replacement for the existing modules. In some use cases there are additional functionality. In a few cases functionality is not present eg hide on child articles as they are redundant with other core features now (or soon to be present)

Expected result AFTER applying this Pull Request

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-5.2-dev labels Jun 3, 2024
@brianteeman brianteeman marked this pull request as ready for review June 3, 2024 15:10
@brianteeman brianteeman requested a review from chmst as a code owner June 3, 2024 15:10
@Fedik Fedik added the Feature label Jun 3, 2024
@Fedik Fedik linked an issue Jun 3, 2024 that may be closed by this pull request
@brianteeman
Copy link
Contributor Author

branch updated to retrigger drone which is failing on unrelated items

@richard67
Copy link
Member

branch updated to retrigger drone which is failing on unrelated items

@brianteeman Not unrelated. See my review comment on the postgresql/base.sql file.

@richard67
Copy link
Member

@brianteeman It needs to add the new module to the extensions helper to this section here https://github.com/joomla/joomla-cms/blob/5.2-dev/libraries/src/Extension/ExtensionHelper.php#L133-L157 , with respect to alphabetical ordering within that section.

@drmenzelit
Copy link
Contributor

I think, there is something missing, you call the layout '_items' here:
<?php require ModuleHelper::getLayoutPath('mod_content', $params->get('layout', 'default') . '_items'); ?>
but there is not such file in the module

@brianteeman
Copy link
Contributor Author

Did you try it?

Did you look at the layouts in other modules and how they are called?

@drmenzelit
Copy link
Contributor

Yes, I tried it. List only works. When I try the other options I get a fatal error
grafik

The module mod_articles_category has a similar layout call, but there is a default_items.php file in the tmpl folder
<?php require ModuleHelper::getLayoutPath('mod_articles_category', $params->get('layout', 'default') . '_items'); ?>

@brianteeman
Copy link
Contributor Author

sorry I was replying from my phone. a file was missing from the commit.

@crimle
Copy link

crimle commented Jun 18, 2024

I have tested this item ✅ successfully on 3a75ffd


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

1 similar comment
@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 3a75ffd


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 18, 2024
@drmenzelit
Copy link
Contributor

With the correct combination one can get almost the same as with the current modules. Only for 2 I was not able to do so:

  • Articles - Archived -> Filter Archived Artcles only
  • Articles - Categories
  • Articles - Category -> Select category, play with parameter, use filter, group them, etc.
  • Articles - Latest -> Change ordering to newest first
  • Articles - Most Read -> Change ordering to hits
  • Articles - Newsflash -> Select category, play with parameter, use filter, group them, etc.
  • Articles - Related

It is possible to select a header level, but that is not being used in the layout
grafik

@brianteeman
Copy link
Contributor Author

  1. Articles Related & Articles Categories - I deliberately didnt include these ones as they're pretty useless and as this new module is an addition instead of a replacement I didnt see the point.

  2. item_heading is a weird one. Will need to look into it further but a quick check shows it is present and used in mod_articles_news and is present and NOT used in mod_article_categories

@drmenzelit
Copy link
Contributor

  1. Thank you for the explanation. I use the two modules sometimes, Articles Related more, although I find it weird that you need to set keywords instead of tags, but that is another story ;-)
  2. It is indeed weird. Maybe we can add "none" as option for cases, where one want to have only a list of titles without a specific heading

@brianteeman
Copy link
Contributor Author

There may be other params that are not presented correctly. More eyes => find more bugs

@pe7er pe7er self-assigned this Jun 25, 2024
@richard67
Copy link
Member

Back to pending due to previous comments which suggest further improvements and completions.


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

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 26, 2024
@pe7er
Copy link
Contributor

pe7er commented Jun 26, 2024

Thank you @brianteeman for this module! And everybody else for the feedback!

Regarding the currently missing “Articles - Categories” and “Articles - Related” views:
We discussed about that in CMS Maintenance Team and decided that we do not need those:

  • The “Articles - Categories” shows Categories and should not be part of the new module.
  • The “Articles - Related” uses some outdated fields which might be removed in Joomla 6.

We would like to have a couple of improvements before we can add it to the core:

  • Because the module does only articles stuff, we would like to re-title the module to mod_articles.
  • We would also like to have item_heading implemented.
  • And we would like to have more layouts so that the users can choose one without having to do a template override.

@brianteeman, do you have time and could you make those improvements?
If not, no problem. Then I can ask someone else to further improve your work.
Thanks!

@brianteeman
Copy link
Contributor Author

I have the time to work on this when you do me the same courtesy and respond to my requests directly to you that you simply ignore.

@chmst
Copy link
Contributor

chmst commented Jun 26, 2024

We had a discussion in the maintainer channel about that, The RTC had to be reverted,.
Testers must know why their tests are rejected.

@richard67
Copy link
Member

@pe7er I think @brianteeman refers to #43625 with his comment #43610 (comment) .

@pe7er
Copy link
Contributor

pe7er commented Jun 27, 2024

I have the time to work on this when you do me the same courtesy and respond to my requests directly to you that you simply ignore.

I'm sorry if you feel ignored with another PR as that wasn't my intention.

I feel your reply as being passive aggressive. When I volunteered for my role as Joomla 5.2 Release Manager, I did not sign up for this kind of negativity. I do not accept this kind of behavior from anyone.
I have just added you to my .joomlaignore list.

@brianteeman
Copy link
Contributor Author

@chmst agreed. As Richard realised I was talking about something else. I will just close this. Don't need or appreciate the comments. Seems that standards of behaviour don't apply to release leads. This is not the first time.

@brianteeman brianteeman deleted the content-module branch June 27, 2024 07:04
@brianteeman brianteeman restored the content-module branch June 30, 2024 08:38
@brianteeman brianteeman deleted the content-module branch June 30, 2024 08:40
@drmenzelit drmenzelit mentioned this pull request Jul 3, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[5.2] Usability: One Articles Module to Rule Them All