User tests: Successful: Unsuccessful:
For the List All Tags
menu type, there are options to set a number of columns. But at present, this does not reflect on the view. This PR will fix this issue and column changes will be reflected on the view.
I consider this as a bug fix as the current menu setting is not reflected in the view. This might be a breaking change, for some websites as the HTML structure will be changed.
This Pull Request will fix a partial of the Issue #39177 .
List All Tags
Number of Columns
from the Options
tabShowing all the tags in a single column without respecting the Number of Columns
in the Options
tab.
Number of Columns
in the Options
tab will be reflected in the view.
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
Category | ⇒ | com_tags Front End |
Status | New | ⇒ | Pending |
Thanks for the pr, can you not make it in a backwards compatible way?
Thanks for your comment. I can't understand what you are saying here. are you asking me to make it backward compatible?
At this moment the number of columns compared with Bootstrap 12 column grid. So for me, using the row-column combination is the right way forward. We may add more options for mobile and tab screen column count and append the classes accordingly.
@nasirkhan
Using an unordered list (<ul>
) makes it possible for screen readers to calculate the amount of list items (<li>
). Convert it to a <div>
makes it an empty box not knowing how many items are in the list. I prefer to use the unordered list and would recommend not to merge this PR as it is right now.
So I'm closing this pr. You can always use an override with your changes. I guess we should tackle the issue in a different way.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-09-11 13:43:25 |
Closed_By | ⇒ | laoneo | |
Labels |
Added:
?
PR-4.4-dev
|
thank you for your thoughtful comments.
I think we @laoneo and @hans2103 none of you understood the issue this pull request solves. There is a bug, which is The Number of Columns
from the Options
tab does not have any effect in the frontend view. This pull request solves this, without changing anything in the admin backend. It continues the logic of 12-column grid and follows the Bootstrap recommended approach to creating rows and columns.
@hans2103 mentioned that the current bug is helpful for better accessibility and his personal preferences in using ul
. If accessibility is the concern that can be solved by using a dedicated aria
tags like aria-colcount
. Why don't you want to use such HTML tags which are created to solve accessibility issues?
@laoneo Can you please open the pull request so that I can add accessibility-related
tags. I believe we should not keep a bug in Joomla core just to respect some personal preferences and force everyone to use overrides, whereas we can make a perfect/ working product.
Although this pull request might solve a current bug, it should not create another.
Do not implement aria attributes when semantic html is suitable.
Why change the HTML when only changing the classNames would be enough?
when columns > 1
list-group
set in <ul>
should be changed into row mod-list
list-group-item
and list-group-item-action
set in <li>
should be changed into col
this will result in the following view:
The borders around each item are gone, but the semantics are kept.
@nasirkhan Might this be a solution ?
@hans2103
Yours one could be a solution.
You want to show list items as a grid by the row
-column
CSS classes which is good to use with divs (as per Bootstrap doc). This list does not look like a list, visually, and it does not have list-related classes and you want to remove the list-group classes as well. And these li
are showing h3
within it. So why are you using li
for?
You said you want to use li
to get the column count for screen readers, whereas the column count value is available on that page. If accessibility is the concern aria-colcount
and aria-rowcount
is the perfect solution for every browser-based and custom screen reader application.
Using ul
-li
and displaying it as a grid might not be a good example of semantic HTML either.
Thanks for the pr, can you not make it in a backwards compatible way?