User tests: Successful: Unsuccessful:
Pull Request for Issue #33042 and #33037 .
Extended existent Javascript (shared-categories-accordion.js
) to a collapse function for categories with subcategories.
Removed Bootstrap collapse attributes
Added a css for the view
See issues
See issues
The collapse function works with and without published menu (see explanation in issue #33042
Status | New | ⇒ | Pending |
Category | ⇒ | Repository NPM Change JavaScript Front End com_content |
Labels |
Added:
NPM Resource Changed
?
|
What you did is also okay.
If you mean bootstrap/collapse.js, it part of Bootstrap 5 changes
https://getbootstrap.com/docs/5.0/components/collapse/
it is possible to solve this with Joomla custom elements?
Yes and No. Yes as the collapse.js will do the accordion thing but No as the repo is basically abandoned. Even the existing elements that Joomla is using have several defects...
@Fedik she means https://github.com/joomla-projects/custom-elements/blob/master/src/js/collapse/collapse.js which in theory should be CSS framework agnostic
@Fedik , @dgrammatiko thank you guys for the answers
@drmenzelit what if... we change it into <detail>
and <summary>
?
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/details
It will collapse out of the box.
what if... we change it into and
We are already doing it in
joomla-cms/build/media_source/system/js/fields/joomla-image-select.w-c.es6.js
Lines 254 to 295 in f4ac188
@drmenzelit what if... we change it into
<detail>
and<summary>
?
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/detailsIt will collapse out of the box.
That is a good idea! I will look into it
summary / detail will not fit in this case, because we need a link to the category too.
I tested this and it worked. So if one of our frontend dev looks at this and confirm that the solution is OK, I will post my test result. Hope it will be merged before next RC.
It looks good to me,
I only think that the CSS added here need to be part of the template style
I think a little bit of CSS is important to make the categories list looks good independently of the template used (we have CSS also on some core modules). We can improve things in Cassiopeia and other templates can use or override the CSS here.
I only think that the CSS added here need to be part of the template style
No, the css should not be monolithic. Joomla is modular and the css/js should be modular
@dgrammatiko So this PR is fine as how it is? Or should the css/js code be moved to system assets so that we can use accordion anywhere we want? If it is fine, I will submit my test result as I tested the feature and it is working well.
Or should the css/js code be moved to system assets so that we can use accordion anywhere we want?
Good question. Turns out that this is the 3rd accordion that Joomla 4 will ship:
But then again if this is working as expected and passes the a11y reqs it's fine
it is possible to solve this with Joomla custom elements?
Yes and No. Yes as the collapse.js will do the accordion thing but No as the repo is basically abandoned. Even the existing elements that Joomla is using have several defects...
@Fedik she means https://github.com/joomla-projects/custom-elements/blob/master/src/js/collapse/collapse.js which in theory should be CSS framework agnostic
@dgrammatiko I asked for Joomla Custom Elements before...
I don't mind, if this PR get rejected, if there is another thing that do the job. I wanted to have a framework agnostic solution....
@dgrammatiko I asked for Joomla Custom Elements before...
I know and I answered that you shouldn't use that code (collapse) which is still true today. What changed is that I rewrote from scratch the Tabs but for this particular instance a plain (ie non CE) implementation is better (ie less code)
The PR is totally fine as is
@drmenzelit since the js file is loaded as a module the code shouldn't use the iife, should be like:
/**
* @copyright (C) 2018 Open Source Matters, Inc. <https://www.joomla.org>
* @license GNU General Public License version 2 or later; see LICENSE.txt
*/
if (!Joomla || !Joomla.Text) {
throw new Error('core.js was not properly initialised');
}
// Selectors used by this script
const buttonsSelector = '[id^=category-btn-]';
/**
* Handle the category toggle button click event
* @param event
*/
const handleCategoryToggleButtonClick = ({ currentTarget }) => {
const button = currentTarget;
const icon = button.querySelector('span');
// Toggle icon class
icon.classList.toggle('icon-plus');
icon.classList.toggle('icon-minus');
// Toggle aria label, aria-expanded
const ariaLabel = button.getAttribute('aria-label');
const ariaExpanded = button.getAttribute('aria-expanded');
button.setAttribute(
'aria-label',
(
ariaLabel === Joomla.Text._('JGLOBAL_EXPAND_CATEGORIES') ? Joomla.Text._('JGLOBAL_COLLAPSE_CATEGORIES') : Joomla.Text._('JGLOBAL_EXPAND_CATEGORIES')
),
);
button.setAttribute(
'aria-expanded',
(
ariaExpanded === 'false' ? 'true' : 'false'
),
);
const target = button.nextElementSibling;
target.toggleAttribute('hidden');
};
Array.from(document.querySelectorAll(buttonsSelector)).forEach((button) => {
button.addEventListener('click', handleCategoryToggleButtonClick);
});
No, the css should not be monolithic. Joomla is modular and the css/js should be modular
If you look in CSS it doing nothing "category specific", it just a general styling like display:flex
and stuff, which I suspect can be done with .d-flex
and other bootstrap. classes.
It not much important to me, I only saying that it can be done without this file
I have modified the JS file as suggested by @dgrammatiko
@drmenzelit you have to push the changes to Github...
@drmenzelit you have to push the changes to Github...
My notebook is driving me crazy today... push is done now
I have tested this item
I don't have good enough frontend skill to confirm the solution is valid or not. But it is working as expected, tested it again today.
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
One question about presets otherwise fine to merge for 4.0.0 as this is core functionality. Thanks for reviewing the JS @dgrammatiko !
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-08-09 10:48:21 |
Closed_By | ⇒ | wilsonge | |
Labels |
Added:
?
?
Removed: ? |
Thanks!
What do you think about removing line 29 in components/com_content/tmpl/categories/default_items.php? Or at least hide it?
We have nowhere else a text for article counts.