Fixing width of Commit list field.
Review Request #11814 — Created Sept. 17, 2021 and submitted
Information | |
---|---|
sandysaji | |
Review Board | |
release-4.0.x | |
4891 | |
Reviewers | |
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:
- Create a review request with a long commit message.
- 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 |
---|
Description | From | Last Updated |
---|---|---|
A few small nits on the description: Typo: "Currenltly" -> "Currently" Descriptions should wrap to about 75 characters, which helps … |
|
|
Can you add two screenshots, one showing the way that it looked before, and one showing how it looks now … |
|
|
Unit tests wouldn't test this sort of thing, so it's probably not worth mentioning the test suite here. Instead, I'd … |
|
|
It doesn't seem this is really fixed. It's more broken in a different way. By applying the overflow rule, we're … |
|
Description: |
|
|||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
-
-
A few small nits on the description:
- Typo: "Currenltly" -> "Currently"
- 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.
- Rather than linking to the issue in the description, put the bug number in the "Bugs" field on the right.
-
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?
-
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.
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||
Bugs: |
|
|||||||||||||||||||||||||||||||||||||||||||||
Added Files: |
-
-
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.
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+3 -1) |
|||||||||||||||||||||||||||||||||||||||||||||
Added Files: |