Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Long forum description breaks the layout as the forum list table can became wider than the page itself.
The reason is the .forum td { whitespace: nowrap; }
rule in forum.theme.css at line 9. It was introduced by #2408513: Refactor forum module CSS files inline with our CSS standards replacing some more specific selectors.
Proposed resolution
Remove the rule or use more specific selectors.
Remaining tasks
Fix the bug.
User interface changes
The forum list table won't overflow even a forum has long description.
API changes
none
Data model changes
none
Beta phase evaluation
Issue category | Bug because it creates visual issues. |
---|---|
Issue priority | Not critical because the forum functions fine. |
Unfrozen changes | Unfrozen because it only changes markup |
Prioritized changes | The main goal of this issue is to visually fix the appearance of a component in Core. |
Disruption | Non-disruptive it fixes a visual bug |
Comment | File | Size | Author |
---|---|---|---|
#5 | forum_before.png | 16.05 KB | rudraram |
#5 | forum_after.png | 19.39 KB | rudraram |
#3 | core-remove_td_nowra-2564857-3p.patch | 412 bytes | bruvers |
screenshot-2015-09-08 13-06-51.png | 35.74 KB | thamas |
Comments
Comment #2
thamasThese are the affected lines of #2408513: Refactor forum module CSS files inline with our CSS standards
The removed selectors were very old part of the Drupal core code (I saw them to exist in commits in 2006…).
However they did not include the first column of the forum list table (used to be
.forum
and called to.forum-list__forum
currently). So the first column became narrower when other column became wider.By the way if we commented our code, we could know what the goal of a rule is and what would be affected by a modification.
Comment #3
bruvers CreditAttribution: bruvers at Wunder commentedThe patch removes the
.forum td { white-space: nowrap; }
rule, as it is a simple fix and I could not find any regressions.Comment #5
rudraram CreditAttribution: rudraram at Axelerant commentedPatch in #3 works good and got applied clean. We can RTBC once the test bot finishes it's job. Attaching screenshots.
Before:
After:
Comment #6
rudraram CreditAttribution: rudraram at Axelerant commentedTest bot failed. that's weird. I think it should be re-tested.
Comment #7
bruvers CreditAttribution: bruvers at Wunder commentedThe patch shouldn't cause any failures. I queued the patch for re-testing.
Comment #8
jaxxed CreditAttribution: jaxxed at Wunder commentedre-test passed, this needs review.
Comment #10
LewisNyman CreditAttribution: LewisNyman at Wunder commentedThanks for the patch and the manual testing. I reviewed the code and I'm happy with this solution. Yay for deleting code.
Comment #11
thamasThank you all for the quick fix!
Comment #12
alexpottYeah but what this issue is not addressing is why the nowrap was there for the other columns in the first place. And it does not say why it is no longer necessary.
Comment #13
LewisNyman CreditAttribution: LewisNyman at Wunder commented@alexpott Good point. I looked into this code and traced it back to a commit made in 2003:
Interesting that back then all CSS was kept within one
drupal.css
file. I guess this was something to do with forum icons? The implementation has probably changed since those days when everyone was still using IE6 :)Comment #14
alexpottWell the fix is certainly an improvement :). Committed bfee1c7 and pushed to 8.0.x. Thanks!