Fixing width of Commit list field.

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

sandysaji
Review Board
release-4.0.x
4891
reviewboard

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
Fixing width of Commit list field.
Wrap longer commit messages within each entry

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. 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. 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. 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. 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
Review request changed

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

  ~ '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.
   
  1. 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.

Commits:

Summary
-
Fixing width of Commit list field.
+
Fixing width of Commit list field.
+
Wrap longer commit messages within each entry

Diff:

Revision 2 (+3 -1)

Show changes

Added Files:

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
Loading...