User tests: Successful: Unsuccessful:
Note: this can be merged after #20626 is merged.
This PR introduces several features and fixes for Articles - Category module.
Publish Articles - Category module. Existing features work like before or like stated above.
Try new options:
Month Yearand try different values for
Date Grouping Field.
New module options added.
New module helper class function added.
Updated existing class function doc blocks.
|Category||⇒||Administration com_modules Language & Strings Modules Front End|
Is this backwards compatible?
A PR really should address one issue only. When you fix things and add new features at the same time it makes it hard to test etc
I'd like to think so.
New options are B/C safe because they need to be enabled to take effect.
Item data is largely unaffected, except for corrected tag loading behavior and grouping by author.
I did remove
$item_heading variable which refers to param that was removed 5 years ago (#1131). For users who use template overrides from 5 years ago this could show undefined variable warning. If this is an issue, this can be reverted and held until 4.0.
I did remove $item_heading variable which refers to param that was removed 5 years ago (#1131). For uqsers who use template overrides from 5 years ago this could show undefined variable warning. If this is an issue, this can be reverted and held until 4.0.
For me that would be a b/c break
"All Categories" hint is missing after applying patch. I think "All Categories" is a welcome hint for users.
That would mean that we change the default placeholder each time we use a
multiple class , i.e. All Tags, All Categories, All Authors, All Access Levels, instead of
I am not in favor of that change.
We could evidently add for each of these fields' description/tip all over Joomla something like
If you do not select anything, then etc. 17 times in present core, this PR included.
I am sure some would oppose strongly to that.
here are the files concerned
~/administrator/components/com_content/models/forms/filter_articles.xml:28: class="multipleCategories" ~/administrator/components/com_content/models/forms/filter_articles.xml:40: class="multipleAccessLevels" ~/administrator/components/com_content/models/forms/filter_articles.xml:50: class="multipleAuthors" ~/administrator/components/com_content/models/forms/filter_articles.xml:73: class="multipleTags" ~/administrator/components/com_content/models/forms/filter_featured.xml:28: class="multipleCategories" ~/administrator/components/com_content/models/forms/filter_featured.xml:53: class="multipleAccessLevels" ~/administrator/components/com_content/models/forms/filter_featured.xml:63: class="multipleAuthors" ~/administrator/components/com_content/models/forms/filter_featured.xml:86: class="multipleTags" ~/components/com_content/models/forms/filter_articles.xml:28: class="multipleCategories" ~/components/com_content/models/forms/filter_articles.xml:40: class="multipleAccessLevels" ~/components/com_content/models/forms/filter_articles.xml:50: class="multipleAuthors" ~/components/com_content/models/forms/filter_articles.xml:87: class="multipleTags" ~/modules/mod_articles_category/mod_articles_category.xml:106: class="multipleCategories" ~/modules/mod_articles_category/mod_articles_category.xml:147: class="multipleTags" ~/modules/mod_articles_category/mod_articles_category.xml:176: class="multipleAuthors" ~/modules/mod_articles_news/mod_articles_news.xml:32: class="multipleCategories" ~/modules/mod_articles_news/mod_articles_news.xml:44: class="multipleTags"
BTW, the default placeholder for Author Aliases is wrong as it proposes to type or select option.
I propose to add a class in the field
and evidently in module edit:
JHtml::_('formbehavior.chosen', '.multipleAliases', null, array('placeholder_text_multiple' => JText::_('JOPTION_SELECT_ALIAS')));
and add the new string in the en-GB.ini files (back and front)
That would mean that we change the default placeholder each time we use a multiple class , i.e. All Tags, All Categories, All Authors, All Access Levels, instead of Select ...
Would be at least helpful whenever we're talking about configuration options. A clear statement instead of may be or may be not "All Thangs"
I am not in favor of that change.
In this case the PR changed that. From my point of view not a bug...
this is why you never try to solve multiple issues in a single pr. it makes it impossible to test
@ReLater using empty string as default value is not OK. It causes incorrect values to be saved. This can be solved in the code (see #20245 for example) but incorrect values shouldn't be passed in the first place. And using filters on such fields causes more issues.
Then there's UX issue. Current implementation is misleading, if not completely wrong. Users can leave default option selected and select additional options. If you have "All Categories" and "Some Category" selected, logically the results should include items from all categories but instead include items from "Some Category". All in all, this wasn't very well thought out.
But those are just my personal comments. See #17668. This is the right way going forward.
This was left out because alias author is a custom SQL field at the moment. It needs to be added as core field but that's for another time.
No need to be added as core field to modify the way we get the placeholder the way I suggested imho. What we have now as default placeholder is just plain wrong.
For UI consistency within this module, yes, the placeholder should be added. But that's the thing, at the moment this placeholder is needed specifically for this module, because author alias is not a SQL field.
Author alias does need to be added as core field because it needs to be reusable and because core shouldn't use SQL fields anyways. If you search for
type="sql" you'll see author alias field in mod_articles_category.xml is the last remaining SQL field.
Anyways, I'm going to take your advice. But with idea of adding author alias field type later on.
To be honest, though, I'm not a fan of this placeholder approach in general. #12447 was the best solution yet, IMO. I'd like to see it revived.
Remains a question: shall we keep or not using the plural forms as the field is multiple?
We do have both variants in en-GB.ini. The plural form has only been used until now in core for this module.
JOPTION_SELECT_AUTHOR_ALIAS="- Select Author Alias -" JOPTION_SELECT_AUTHOR_ALIASES="- Select Author Aliases -" JOPTION_SELECT_AUTHOR="- Select Author -" JOPTION_SELECT_AUTHORS="- Select Authors -"
The singular ones are commonly used when filtering in the managers.
The singular ones are commonly used when filtering in the managers.
Consistency wins so i would go with the singular. I would only use the plural in a place where it has to be a multiple
Problem loading page with error
The connection was reset in Firefox with nightly build.
Test English (GB) Sample Data
Display Options, show
@Quy do you install sample data during Joomla installation? Because I don't see
Test English (GB) Sample Data option. Only these:
None (Required for basic native multilingual site creation) Blog English (GB) Sample Data Brochure English (GB) Sample Data Default English (GB) Sample Data Learn Joomla English (GB) Sample Data
Tried with 3.8.8-dev and 3.9.0-dev.
@SharkyKZ Yes during Joomla installation with staging using XAMPP:
When debug is on, I get this.
Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 4096 bytes) in \libraries\src\Log\LogEntry.php on line 116 Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 20480 bytes) in \libraries\vendor\joomla\registry\src\Registry.php on line 85
I will test it on a live server to see if it can be reproduced.
We get test sample data by installing this:
@infograf768 thanks for the link.
@Quy you're getting an infinite loop.
Articles Category Module article contains
Articles Category module (using Load Module plugin).
Articles Category module displays the article.
When introtext is enabled in the module, the module then displays the article text which contains the module.
So the module tries to render itself causing an infinite loop.
The issue isn't caused by this patch but in your specific case, the fixes in the patch allows the article containing the module to appear in the module. Before the patch, the articles would have been incorrectly filtered out.
For tags, it is missing
itemscope itemtype="https://schema.org/Article" properties. Are they necessary?
<div class="mod-articles-category-tags"> <ul class="tags inline"> <li class="tag-2 tag-list0" itemprop="keywords"> <a href="/joomla-cms-staging/tagged-items" class="label label-info"> Red </a> </li> <li class="tag-3 tag-list1" itemprop="keywords"> <a href="/joomla-cms-staging/compact-tagged" class="label label-info"> Yellow </a> </li> <li class="tag-4 tag-list2" itemprop="keywords"> <a href="/joomla-cms-staging/all-tags/green" class="label label-info"> Green </a> </li> </ul> </div>
What are Joomla's standards for microdata? Should Google's Sructured Data Testing Tool be used for validation? If so, adding Article itemtype would only result in multiple validation errors due to missing data (the same is true for article views and other modules). Meanwhile, loose itemprop attributes show no errors.
Because of the addition of
showon="date_filtering:range" (which is great) the following strings need updating and simplifying. We don't need the "If xxx is selected above"
MOD_ARTICLES_CATEGORY_FIELD_ENDDATE_DESC="If Date Range is selected above, please enter an End Date." MOD_ARTICLES_CATEGORY_FIELD_RELATIVEDATE_DESC="If Relative Date is selected above, please enter in a numeric day value. Results will be retrieved relative to the current date and the value you enter." MOD_ARTICLES_CATEGORY_FIELD_STARTDATE_DESC="If Date Range is selected above, please enter a Starting Date."
Sorry not wasting any more time on this - there are far too many unrelated changes to make testing sensible and its impossible to follow comments. I can see there is some good stuff here but I also see other changes which have impact elsewhere.
This is why you must have one pr per issue
|Closed_Date||0000-00-00 00:00:00||⇒||2018-06-22 08:45:45|