Summary

Update diff "Start Over" text is now inside of patch dialog box

Review Request #7871 — Created Jan. 15, 2016 and submitted Jan. 21, 2016, 12:45 p.m.

Information

Owner

samchurney

Repository

Review Board

Branch

release-2.5.x

Bugs

4034

Depends On

Reviewers

Groups

reviewboard, students

People

Description

The "Start Over" text in the Update diff dialog was floating in
free space below the box. This was due to huge line spacing.
Now the "Start Over" text sits in the bottom right-hand corner of
the dialog box.

Similarly, the contents of the dialog box were off centre and were
touching the border, which looked bad.
Now the contents are centered both vertically and horizontally
which looks much nicer.

Testing Done

These changes were tested in Chrome 47, Firefox 43, and Safari 9
by viewing them in each browser and nothing seemed broken.

The bug could not be reproduced with these changes implemented.

Files

Issues

Description	From	Last Updated
There's a bunch of trailing whitespace in here.	david	Jan. 16, 2016, 7:50 a.m.
calc is experimental and we can't use it. We should instead use margins.	david	Jan. 16, 2016, 7:49 a.m.
transform is also experimental. We should use a sized element and margin: auto to do the centering.	david	Jan. 16, 2016, 7:49 a.m.

Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/views/updateDiffView.js
    reviewboard/static/rb/css/pages/reviews.less



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/views/updateDiffView.js
    reviewboard/static/rb/css/pages/reviews.less

Description:

		The "Start Over" text in the Update diff dialog was floating in
		free space below the box. This was due to huge line spacing.
		Now the "Start Over" text sits in the bottom right-hand corner of
		the dialog box.

		Similarly, the contents of the dialog box were off centre and were
		touching the border, which looked bad.
		Now the contents are centered both vertically and horizontally
		which looks much nicer.
-
-		These changes were tested in Chrome 47, Firefox 43, and Safari 9
-		by viewing them in each browser and nothing seemed broken.
-
-		The bug could not be reproduced with these changes implemented.

Testing Done:

	+	These changes were tested in Chrome 47, Firefox 43, and Safari 9
	+	by viewing them in each browser and nothing seemed broken.
	+
	+	The bug could not be reproduced with these changes implemented.

Can you add the bug number to this?

samchurney Jan. 15, 2016, 1:35 p.m.

4034: "Start Over" text is outside of patch dialog box

brennie Jan. 15, 2016, 8:52 p.m.

The number (4034) should go in the bugs field.

reviewboard/static/rb/css/pages/reviews.less (Diff revision 1)

The issue has been resolved. Show all issues

There's a bunch of trailing whitespace in here.

reviewboard/static/rb/css/pages/reviews.less (Diff revision 1)

The issue has been resolved. Show all issues

calc is experimental and we can't use it. We should instead use margins.

mike_conley Jan. 15, 2016, 1:35 p.m.

Are you sure? According to the MDN doc, it's supported in:

Firefox 16+
Chrome 26+
IE9+
Safari 7+

david Jan. 15, 2016, 1:38 p.m.

It also says "this technology's specification has not stabilized"

mike_conley Jan. 15, 2016, 1:40 p.m.
```
Fair enough! :D
```

reviewboard/static/rb/css/pages/reviews.less (Diff revision 1)

The issue has been resolved. Show all issues

transform is also experimental. We should use a sized element and margin: auto to do the centering.

Would you mind adding screenshots of the before and after for the dialog? Screenshots are really helpful for UI changes.

Status: Discarded

Status: Re-opened

Diff:

Revision 2 (+3 -1)

Show changes

	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/views/updateDiffView.js

Added Files:

Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/views/updateDiffView.js
    reviewboard/static/rb/css/pages/reviews.less



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/views/updateDiffView.js
    reviewboard/static/rb/css/pages/reviews.less

Change Summary:

Added bug number (4034) to review.

Bugs:

+	4034

Groups:

+	students

Ship it!

```
Looks good to me! Thanks!
```

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-2.5.x (87072a4)