Problem/Motivation

Long forum description breaks the layout as the forum list table can became wider than the page itself.

Long forum description makes the page overflow horizontally

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

Reference: https://www.drupal.org/core/beta-changes
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
CommentFileSizeAuthor
#5 forum_before.png16.05 KBrudraram
#5 forum_after.png19.39 KBrudraram
#3 core-remove_td_nowra-2564857-3p.patch412 bytesbruvers
screenshot-2015-09-08 13-06-51.png35.74 KBthamas

Comments

thamas created an issue. See original summary.

thamas’s picture

thamas
Primary language Hungarian
Location Hungary
commented

These are the affected lines of #2408513: Refactor forum module CSS files inline with our CSS standards

-#forum td.created,
-#forum td.posts,
-#forum td.topics,
-#forum td.last-reply,
-#forum td.replies,
-#forum td.pager {
+.forum td {
   white-space: nowrap;
 }

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.

bruvers’s picture

bruvers commented
Status: Active » Needs review
StatusFileSize
new core-remove_td_nowra-2564857-3p.patch412 bytes

The patch removes the .forum td { white-space: nowrap; } rule, as it is a simple fix and I could not find any regressions.

Status: Needs review » Needs work

The last submitted patch, 3: core-remove_td_nowra-2564857-3p.patch, failed testing.

rudraram’s picture

rudraram commented
StatusFileSize
new forum_after.png19.39 KB
new forum_before.png16.05 KB

Patch in #3 works good and got applied clean. We can RTBC once the test bot finishes it's job. Attaching screenshots.

Before:
forum_before

After:
forum_after

rudraram’s picture

rudraram commented

Test bot failed. that's weird. I think it should be re-tested.

bruvers’s picture

bruvers commented

The patch shouldn't cause any failures. I queued the patch for re-testing.

jaxxed’s picture

jaxxed commented
Status: Needs work » Needs review

re-test passed, this needs review.

jaxxed queued 3: core-remove_td_nowra-2564857-3p.patch for re-testing.

lewisnyman’s picture

lewisnyman
he/him
commented
Status: Needs review » Reviewed & tested by the community

Thanks for the patch and the manual testing. I reviewed the code and I'm happy with this solution. Yay for deleting code.

thamas’s picture

thamas
Primary language Hungarian
Location Hungary
commented

Thank you all for the quick fix!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Yeah 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.

lewisnyman’s picture

lewisnyman
he/him
commented
Status: Needs work » Reviewed & tested by the community

@alexpott Good point. I looked into this code and traced it back to a commit made in 2003:

commit 8a2d5bede9a25af940fc5be248e81911fe056328
Author: Dries Buytaert 
Date:   Sat Aug 16 05:49:45 2003 +0000

    - Rewrote handling of forum icons and added default icons made by Steven
      Wittens.
    
    - Renamed some column titles in the forum module as per Moshe's suggestion.
    
    - Introduced a 'more-link' class to position the 'more' links.  Removed
      hard-coded markup from the modules.
    
    - Fixed bug in import module: the theme functions called a non-existing
      function.

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 :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Well the fix is certainly an improvement :). Committed bfee1c7 and pushed to 8.0.x. Thanks!

  • alexpott committed bfee1c7 on 8.0.x 
    Issue #2564857 by bruvers, rudraram, thamas, LewisNyman: Long forum...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.