Summary:

Fixes the blank space visible in the infobox when avatars are disabled.

Review Request #10160 — Created Sept. 21, 2018 and submitted Oct. 15, 2018, 11:53 a.m.

Information
Owner:	Sudolicious
Repository:	Review Board
Branch:	release-4.0.x
Bugs:	4738
Depends On:
Commit:	7130f52...
Reviewers
Groups:	reviewboard, students
People:

Description

When avatars are disabled a blank space was visible on the infobox
where the avatar should be.

The code before checked if the avatar service was enabled and would
then make a request to gravatar. Now the code will check if avatar
services are enabled and if there is an avatar set.
If the services are disabled you will there will be no space
for an avatar. Otherwise, you will see a default avatar.

Testing Done

Python and JavaScript tests were passed.

Tested inside of Firefox and Chrome in 2 scenarios
1. Enabling avatars
2. Disabling avatars

Files

Issues

Description	From	Last Updated
Please see screenshot number 3 below Thanks for supplying screenshots! That really clears up what the end user experience will …	mike_conley	Sept. 22, 2018, 8:25 a.m.
Can you update the summary to use proper capitalization? ("the infobox" -> "The infobox").	david	Sept. 22, 2018, 8:26 a.m.
Please put the associated bug number into the "Bugs" field.	david	Sept. 22, 2018, 8:26 a.m.
Please wrap your description to 72 columns (this gets turned into the commit message)	david	Sept. 28, 2018, 1:25 p.m.
Can you write your commit summary in the imperitive mood, i.e., as if it were a command. The following sentence …	brennie	Sept. 28, 2018, 1:36 p.m.
Please undo all the indentation changes your editor auto-"fixed".	brennie	Sept. 28, 2018, 2:03 p.m.
I think we have a convention of keeping the template tags that open on a newline unindented, but indenting the …	mike_conley	Sept. 28, 2018, 2:03 p.m.

Status: Discarded

Status: Re-opened

Description:

~		the infobox has been updated so that it doesn't display a blank space on the avatar
	~	When avatars are disabled a blank space was visible on the infobox where the avatar should be. Please see screenshot number 3 below.
	+
	+	The code before checked if the avatar service was enabled and would then make a request to gravatar. Now the code will check if avatar services are enabled and if there is an avatar set. If the services are disabled you will see screenshot 1. If avatar services are enable then you will see Screenshot 2.

Testing Done:

	+	Python and JavaScript tests were passed.
	+
	+	Tested inside of Firefox and Chrome in 2 scenarios
	+	1. Enabling avatars
	+	2. Disabling avatars

Commit:

+	b5ea5263150e31528702b61266dfe69099339fa7

Diff:

Revision 1 (+54 -47)

	reviewboard/accounts/views.py
	reviewboard/templates/accounts/user_infobox.html

Added Files:

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Thanks! This looks like a solid solution, John. I just have a few suggestions - see below.

Show all issues

Please see screenshot number 3 below

Thanks for supplying screenshots! That really clears up what the end user experience will be like.

However, we use the Description and Testing Done sections to construct the final commit messages, and the commit will not include those screenshots. Can you please remove the reference to screenshots (since it won't be available in the commit message)?

reviewboard/templates/accounts/user_infobox.html (Diff revision 1)

Show all issues

I think we have a convention of keeping the template tags that open on a newline unindented, but indenting the internal bits. So I think we prefer:

{% if foo %}
{%   if bar %}
{%   endif %}
{% endif %}


instead of:

{% if foo %}
  {% if bar %}
  {% endif %}
{% endif %}


So can you please reformat this file to the original indentation style?

Show all issues

Can you update the summary to use proper capitalization? ("the infobox" -> "The infobox").

Show all issues

Please put the associated bug number into the "Bugs" field.

Show all issues

Please wrap your description to 72 columns (this gets turned into the commit message)

Sudolicious Sept. 22, 2018, 8:30 a.m.

Is it wrapped to 72 columns or 72 characters?

david Sept. 22, 2018, 8:33 a.m.

72 according to https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html

Summary:

-	the infobox has been updated so that it doesn't display a blank space on the avatar
+	The infobox has been updated so that it doesn't display a blank space on the avatar

Description:

~		When avatars are disabled a blank space was visible on the infobox where the avatar should be. Please see screenshot number 3 below.
	~	When avatars are disabled a blank space was visible on the infobox where the avatar should be.

~		The code before checked if the avatar service was enabled and would then make a request to gravatar. Now the code will check if avatar services are enabled and if there is an avatar set. If the services are disabled you will see screenshot 1. If avatar services are enable then you will see Screenshot 2.
	~	The code before checked if the avatar service was enabled and would then make a request to gravatar. Now the code will check if avatar services are enabled and if there is an avatar set. If the services are disabled you will there will be no space for an avatar. Otherwise, you will see a default avatar.

Bugs:

+	4738

Description:

~		When avatars are disabled a blank space was visible on the infobox where the avatar should be.
	~	When avatars are disabled a blank space was visible on the infobox
	+	where the avatar should be.

~		The code before checked if the avatar service was enabled and would then make a request to gravatar. Now the code will check if avatar services are enabled and if there is an avatar set. If the services are disabled you will there will be no space for an avatar. Otherwise, you will see a default avatar.
	~	The code before checked if the avatar service was enabled and would
	+	then make a request to gravatar. Now the code will check if avatar
	+	services are enabled and if there is an avatar set.
	+	If the services are disabled you will there will be no space
	+	for an avatar. Otherwise, you will see a default avatar.

Show all issues

Can you write your commit summary in the imperitive mood, i.e., as if it were a command.

The following sentence should make sense if you substitute your summary in:

"This patch will <summary>"

reviewboard/templates/accounts/user_infobox.html (Diff revision 1)

Show all issues

Please undo all the indentation changes your editor auto-"fixed".

Summary:

-	The infobox has been updated so that it doesn't display a blank space on the avatar
+	Fixes the blank space visible in the infobox when avatars are disabled.

Change Summary:

Fixes the spacing that my editor 'auot-fixed'

Commit:

-	b5ea5263150e31528702b61266dfe69099339fa7
+	7130f52d0c70646848ed881652ef21983beba2ba

Diff:

Revision 2 (+8 -2)

Show changes

	reviewboard/accounts/views.py
	reviewboard/templates/accounts/user_infobox.html

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

```
Ship It!
```

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (812d949f)