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

[3.9-alpha] [mod_articles_category] Make sure the tag does not conflict with the title of the article #21937

Merged
merged 2 commits into from
Sep 8, 2018

Conversation

zero-24
Copy link
Contributor

@zero-24 zero-24 commented Aug 31, 2018

Pull Request for Issue #21083

Summary of Changes

I have moved the translation part to the view so the code does not work different on different languages. As currently the array key is the translated word already and in that case have different results depending on the used language this patch makes sure the result is always the same on all languages by translating the key later.

Testing Instructions

  • install 3.9-alpha (with sample testing data)
  • create and mod_articles_category with the new option (Group by tags) added here [mod_articles_category] Option to group by tags #21083
  • Check that is displays good in the frontend
  • Create an article with the Tag "Untagged"
  • Notice that the article is listed in the Untagged list in the frontend.
  • apply this patch
  • Joomla differentiate now between actually tagged and not tagged articles.

Expected result

Joomla differentiate now between actually tagged and not tagged articles.

Actual result

Joomla does not differentiate between actually tagged and not tagged articles.

Documentation Changes Required

None.

cc @SharkyKZ as original author of the PR #21083

@zero-24 zero-24 added this to the Joomla 3.9.0 milestone Aug 31, 2018
@SharkyKZ
Copy link
Contributor

This breaks ordering.

@zero-24
Copy link
Contributor Author

zero-24 commented Sep 1, 2018

What do you mean? Without this the ordering is always different per language and can result into false positives when the article title is the same like the translation. Not to forget some maybe illegal chars for an array key in the translation.

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Sep 1, 2018

Alphabetical ordering. Untagged will always be where M should be because of language key. I guess, because it's not a real tag, we could just append it to the list. See zero-24#39 for possible solution.

@zero-24
Copy link
Contributor Author

zero-24 commented Sep 1, 2018

I have merged your PR. Thanks!

@crommie
Copy link

crommie commented Sep 8, 2018

Not clear what should happen here. Without patch: list is rendered, with the new tagged article at the bottom of the list.

afbeelding

With patch: list is rendered, with the new tagged article at the top of the list.

afbeelding

Is this the expected outcome?


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

@sanderpotjer
Copy link
Member

I have tested this item ✅ successfully on cc3be04

With patch the article is listed under the label "Untagged"
After patch the article is listed under the label "Untagged", which is now a real tag heading. Other articles are grouped with "Untagged" at end of the list.


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

@crommie
Copy link

crommie commented Sep 8, 2018

I have tested this item ✅ successfully on cc3be04

Ah. Tag needed a capital to render expected outcome.

Without patch: tagged article appears in Untagged items
With patch: tagged article appears in items with tag Untagged.


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

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.9.0 milestone Sep 8, 2018
@ghost
Copy link

ghost commented Sep 8, 2018

Ready to Commit after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 8, 2018
@hildaabbing
Copy link

I have tested this item ✅ successfully on cc3be04

Steps reproduced. Works as expected


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

@mbabker mbabker added this to the Joomla 3.9.0 milestone Sep 8, 2018
@mbabker mbabker merged commit 8b3130c into joomla:3.9-dev Sep 8, 2018
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 8, 2018
@zero-24
Copy link
Contributor Author

zero-24 commented Sep 16, 2018

Thanks 👍

@zero-24 zero-24 deleted the modarticescategoryuntagged branch September 16, 2018 15:49
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.

7 participants