[#9388] - Missing class ul tag
- Closed
- 13 May 2017
- Medium
- Build: staging
- # 9388
- Diff
- flash1452:missing-class-ul-tag
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #8945 .
Summary of Changes
Introduced a new class in the <ul> tag of file mod_tags_popular and mod_tags_similar files.
Testing Instructions
The tag was a simple <ul> tag
Now its: <ul class="tagspopular">
| Status | New | ⇒ | Pending |
| Labels |
Added:
?
|
||
brianteeman
- | Category | ⇒ | Modules |
| Rel_Number | 0 | ⇒ | 8945 |
| Relation Type | ⇒ | Pull Request for |
This PR has received new commits.
CC: @Twincarb
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9388.
I have tested this item 
successfully on 95b25c7
Tested both mod_tags_popular and mod_tags_similar both showing new ul class
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9388.
@brianteeman Please review my PR.
I have tested this item successfully on 95b25c7
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9388.
This PR has received new commits.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9388.
As there already is a class on the surrounding div (tagspopular / tagssimilar) you could address the ul like this: .tagssimilar > ul in your css. So I think there is no need for a class on the ul - keeps code cleaner.
I have tested this item
Patch does add mentioned class to <ul> of popular tags.
But as @lunalars mentioned, the parent div already has a appropriated class to select the tags.
Post-patch there will be a parent <div class="tagspopular"> with a child <ul class="tagspopular"> which seems messy.
Hosted by @icampus
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9388.
I have tested this item
tested @icampus
patch does what it says. adds a class=tagspopular to the
- tag of popular tags
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9388.
| Status | Pending | ⇒ | Ready to Commit |
Ready to commit as we have 2 successful tests.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9388.
| Category | Modules | ⇒ | Front End Modules |
| Labels |
Added:
?
|
||
Removing RTC label - the div and ul element shouldn't have the same class as has been pointed out.
| Status | Ready to Commit | ⇒ | Pending |
| Labels | |||
| Labels |
Removed:
?
|
||
franz-wohlkoenig
- | Status | Pending | ⇒ | Needs Review |
We can't fix this in a backwards compatible way because you can't remove the class on the div as it may break existing sites and we can't add the class to the ul as it would be a duplicate class. A real catch-22, only way I see this fixed is in Joomla 4. Thanks for your effort.
| Status | Needs Review | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-05-13 20:19:40 |
| Closed_By | ⇒ | roland-d |
I have tested this item
successfully on 10d956f
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9388.