Fixing width of Commit list field.

Review Request #11814 — Created Sept. 17, 2021 and submitted

Information

Review Board
release-4.0.x

Reviewers

Currently, longer commit messages in a review request go beyond the page width.
Using a css fix, the commit address this issue.

I have attached two files (Screenshots)

'Beyond_page.png': Showing the way it looked before the fix
'Within_page.png': Showing the way it looks with the fix.
(Update) 'Wrapped_message.png': Screenshot of the latest fix where long texts are wrapped.

Tested on Chrome on MacOS.

Instruciton to replicate the issue:

  1. Create a review request with a long commit message.
  2. Make your web brower window's width shorter than the summary length in "Commits" section.

Expected result: The longer commit messages does not go past the page width.

Summary ID
Fixing width of Commit list field.
14d7a8f6097df54bdc2335c3659e3347ea156853
Wrap longer commit messages within each entry
d345a42d2afd5dd916d1e95398903083cecb2ea7

Description From Last Updated

A few small nits on the description: Typo: "Currenltly" -> "Currently" Descriptions should wrap to about 75 characters, which helps …

chipx86chipx86

Can you add two screenshots, one showing the way that it looked before, and one showing how it looks now …

chipx86chipx86

Unit tests wouldn't test this sort of thing, so it's probably not worth mentioning the test suite here. Instead, I'd …

chipx86chipx86

It doesn't seem this is really fixed. It's more broken in a different way. By applying the overflow rule, we're …

chipx86chipx86
sandysaji
sandysaji
chipx86
  1. 
      
  2. Show all issues

    A few small nits on the description:

    1. Typo: "Currenltly" -> "Currently"
    2. Descriptions should wrap to about 75 characters, which helps them fit properly in Git commit messages. Often, the editor used to create commit messages will wrap at the necessary point (though it depends on your environment), so we recommend drafting the description there.
    3. Rather than linking to the issue in the description, put the bug number in the "Bugs" field on the right.
    1. Done! Updated the typo, shortened the description, and added bug number in the bugs field.

  3. Show all issues

    Can you add two screenshots, one showing the way that it looked before, and one showing how it looks now with the fix in place?

    1. Done! I have attached two files (Screenshots)

      'Beyond_page.png': Showing the way it looked before the fix
      'Within_page.png': Showing the way it looks with the fix.

  4. Show all issues

    Unit tests wouldn't test this sort of thing, so it's probably not worth mentioning the test suite here. Instead, I'd like to see more about the manual tests you performed, with enough instruction to let us reproduce the problems and the solutions for when we go to land this.

    1. Updated 'Testing' Section

  5. 
      
sandysaji
chipx86
  1. 
      
  2. Show all issues

    It doesn't seem this is really fixed. It's more broken in a different way. By applying the overflow rule, we're telling the browser to chop off anything that's too wide, but the problem is that it's getting to be too wide in the first place.

    In other words, we're attempting to address the symptom without addressing the core problem.

    I think you'll want to jump back in and investigate that core problem, which is that long text within each entry doesn't wrap. For that, you might want to look at white-space: pre-wrap, which is like <pre>'s default whitespace-preserving behavior (which this will use, being in a <pre>, but with wrapping.

    1. Thanks for explaining that out! white-space: pre-wrap did the trick.
      I have updated the review request with the new fix and screenshot. The longer commit messages now wraps.

  3. 
      
sandysaji
sandysaji
Review request changed

Status: Closed (submitted)

Loading...