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

Articles Module #43738

Merged
merged 43 commits into from
Aug 30, 2024
Merged

Articles Module #43738

merged 43 commits into from
Aug 30, 2024

Conversation

drmenzelit
Copy link
Contributor

@drmenzelit drmenzelit commented Jul 3, 2024

Pull Request for Issue #43404 .

Summary of Changes

This PR is a follow-up of #43610 that was closed some days ago.

I did several changes to the code, added some parameters (for layout) and functions (include specific articles by ID) and created a CSS file to control the layout. After this get merged additional changes in the CSS from Cassiopeia can be done.

Testing Instructions

I you don't directly install the PR package, you will need to run npm (npm run build:css) to get the CSS file.

With this module you can replace the existing mod_articles_xx. All you need is to play with the parameters.

Articles - Archived -> Filter Archived Articles only
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.

Toggle the inline help to get more information about some fields.

Expected result AFTER applying this Pull Request

Screenshot 2024-07-03 at 12-13-02 Typography

Screenshot 2024-07-03 at 12-13-22 Home

Screenshot 2024-07-03 at 12-14-06 Home

Screenshot 2024-07-03 at 13-37-26 Typography

Screenshot 2024-07-03 at 12-11-24 Joomla

Screenshot 2024-07-03 at 12-12-04 Joomla

Screenshot 2024-07-03 at 12-12-31 Typography

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

@drmenzelit drmenzelit requested a review from chmst as a code owner July 3, 2024 12:24
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.2-dev labels Jul 3, 2024
@brianteeman
Copy link
Contributor

Layout horizontal only works in some module positions. Is this intentional? Took me a while to work out why i couldnt get the columns to work

@drmenzelit
Copy link
Contributor Author

Layout horizontal only works in some module positions. Is this intentional? Took me a while to work out why i couldnt get the columns to work

No, no intentional at all... But the number of columns is related to the width of the container (maybe something for the description of the field). For example you can set 3 columns and in main-bottom you will get 2, because the container is smaller than 992px

@brianteeman
Copy link
Contributor

brianteeman commented Jul 3, 2024

The label "card style" confused me - I thought it referred to placing a border around the article. Not that it made the entire article a link,

When Card style is enable you should not be able to make the category a link as it will never work.

Same is true for the tags

@brianteeman
Copy link
Contributor

Articles Info Layout

I think it makes more sense to describe this as single line and multiline as opposed to horizontal and vertical

@drmenzelit
Copy link
Contributor Author

I think it makes more sense to describe this as single line and multiline as opposed to horizontal and vertical

Good idea! Thank you

@brianteeman
Copy link
Contributor

No matter what options you chose it is always output as a list. When you just have the article titles that makes sense but I dont believe it is correct to display articles with content in this way and it should be a div. We dont display a content blog view as a list

@drmenzelit
Copy link
Contributor Author

The label "card style" confused me - I thought it referred to placing a border around the article. Not that it made the entire article a link,

I understand card style as a combination of the two things. I was not sure, if it is good to put so much css on the module itself and better do it in Cassiopeia (as is now in the case of mod_articles_newsflash).

When Card style is enable you should not be able to make the category a link as it will never work.

Same is true for the tags

That is true ... Maybe it will be better to remove the possibility to display category and tags if card style is selected ... It is always difficult to get all ideas working well (and I have a lot of other ideas ;-) )

@drmenzelit
Copy link
Contributor Author

No matter what options you chose it is always output as a list. When you just have the article titles that makes sense but I dont believe it is correct to display articles with content in this way and it should be a div. We dont display a content blog view as a list

I was following a tutorial for accessible cards: https://gehirngerecht.digital/barrierefreie-cards-erstellen-schritt-fuer-schritt-anleitung/
They write: "A list is an advantage for screen readers because, among other things, it tells them how many elements to expect. "

@Fedik Fedik added the Feature label Jul 3, 2024
@brianteeman
Copy link
Contributor

They write: "A list is an advantage for screen readers because, among other things, it tells them how many elements to expect.

That's true but it would also be true for all the blog article views. For me it is an abuse of the list semantics

@brianteeman
Copy link
Contributor

Show Introtext
image

I suggest making this default to Show. Otherwise it is very confusing when you change Titles Only to No and you still dont see the introtext
image

@brianteeman
Copy link
Contributor

Articles Info Layout showon syntax is not correct

showon="title_only:0[AND]show_date:1[OR]show_category:1[OR]show_hits:1[OR]show_author:1"

It should not be displayed when title only = 1

image

Copy link
Contributor

@bembelimen bembelimen left a comment

Choose a reason for hiding this comment

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

I hope this great idea will make it into 5.2, but I also think there is some improvements needed before we can add it. For example we should not add old rich snippets elements. I also think the layout should be a bit cleaned out (I will create created a PR for this against your branch @drmenzelit drmenzelit#38)

@drmenzelit
Copy link
Contributor Author

Merged the changes done by @bembelimen (thank you for cleaning the code) and corrected an error in the logic of displaying article imagas that was interfering with the truncation of the intro text.

@richard67
Copy link
Member

@drmenzelit Could you fix PHPCS errors reported here https://ci.joomla.org/joomla/joomla-cms/78353/1/7 ? Thanks in advance.

@drmenzelit
Copy link
Contributor Author

@richard67 I was on it, done ... I hope ;-)

@pcombet-adosis
Copy link

Hello,
tested in Multilanguage environment (FR/ENG), very interesting feature. Works well. One notice : "Article IDs to Exclude" - I did not manage how to separate the differents articles, tested, comma (,), dash (-) ... until I saw the multiline icon to show it needs a carriage return. It would be nice just to add an info at the RollOver on the "Article IDs to Exclude" text which would say "article ID separated by a carriage return" .
Best
Philippe

@pcombet-adosis
Copy link

I have tested this item ✅ successfully on 56845d4

Hello,
tested in Multilanguage environment (FR/ENG), very interesting feature. Works well. One notice : "Article IDs to Exclude" - I did not manage how to separate the differents articles, tested, comma (,), dash (-) ... until I saw the multiline icon to show it needs a carriage return. It would be nice just to add an info at the RollOver on the "Article IDs to Exclude" text which would say "article ID separated by a carriage return" .
Best
Philippe


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

@JeffOnWire
Copy link

@RickR2H I tested this morning and was unable to recreate the introtext issue I raised on August 23. I have no issues. Everything looks fine.

@JeffOnWire
Copy link

I'm working with Ordering Options today and may have another issue—order by alias seems to be ordering by title. Here's my setup:

Removed configuration.php from the root.
Created a new database, configured Joomla and logged in as admin.
From the administrator dashboard I see a Sample Data module. Installed Testing Sample Data
Created two new user accounts, for Arthur Able and Peter Petra.
Went to Content > Articles, sorted by ID ascending and edited articles with ID 1-6

  • Administrator Components (ID 1): Created by = Arthur Able; leave alias blank
  • Archive Module (ID 2): Alias = Barbara Brown
  • Article Categories Module (ID 3): Created by = Peter Petra; leave alias blank
  • Articles Category Module (ID 4): Alias = Eddie Earnest
  • Authentication (ID 5): No change (created by you); leave alias blank
  • Australian Parks (ID 6): Alias = Helen Hurry

Install the Articles module and configure:

Title: Articles Module Test
Articles to Display: 6
Position: sidebar-right
Article IDs to Include: 1,2,3,4,5,6 on separate lines

Display Options: Title Only = No; Author: Show

Ordering Options: Alias Ascending (and switched back and forth between Ascending and Descending while testing)

I'm observing that the articles are always ordered by Title, regardless of the Author or Alias value.

Question about how Author and Alias

In Display Options we have Author (which will display the Created by Alias if defined in the article, otherwise it displays the Created by user), but in Ordering Options we have Alias, which I'm assuming would function in a similar way...ordering by either Created by or Created by Alias. Should these be labeled the same, either Author or Alias, to be consistent?

Finish Publishing Date

In Ordering Options we have the option to order by Finish Publishing Date but there is no display option for the finish publishing date. Should there be?

@drmenzelit
Copy link
Contributor Author

@JeffOnWire I think there is a misunderstanding of Alias ... Alias refers to the alias of the article, this one on the right:
grafik

@JeffOnWire
Copy link

@drmenzelit Ah, thank you.

@Hackwar Hackwar enabled auto-merge (squash) August 30, 2024 19:38
@Hackwar Hackwar added this to the Joomla! 5.2.0 milestone Aug 30, 2024
@Hackwar
Copy link
Member

Hackwar commented Aug 30, 2024

Thank you @drmenzelit for this great feature!

@Hackwar Hackwar dismissed bembelimen’s stale review August 30, 2024 19:46

Changes have been incorporated.

@Hackwar Hackwar merged commit 8de07cc into joomla:5.2-dev Aug 30, 2024
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 30, 2024
@Kostelano
Copy link
Contributor

Kostelano commented Sep 16, 2024

Sorry, so it doesn't get lost, I created #44088.

@drmenzelit
Copy link
Contributor Author

@Kostelano thank you for the hint, we removed some parameters on the last version of the module before the merge and it seems we forgot to chech this one. We will correct it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.2-dev Unit/System Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.