Summary

Improve the look and implementation of trophies.

Review Request #8324 — Created Aug. 12, 2016 and submitted Aug. 20, 2016, 1:57 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Commit

99fa716...

Reviewers

Groups

reviewboard

People

Description

This change makes a small handful of improvements to our trophies,
covering the implementation of TrophyType subclasses, the presentation
of trophies, and the look of our standard trophies.

On an implementation level, TrophyType subclasses no longer need to
override __init__, as the metadata for the trophy is now stored in class
attributes. Older subclasses can install provide these through the
constructor, but will get deprecation warnings.

Trophy registration is now based on a standard registry. The existing
methods remain for compatibility, but will show deprecation warnings.

All methods and attributes are now properly documented as well.

On a presentational level, rendering problems with showing multiple
trophies or when there's an unknown trophy have been fixed. Trophies are
also no longer bound to a specific width and height (previously, this
would result in some weird problems with the positioning of the trophy).

Trophies can also provide multiple images for different resolution
levels, allowing for @2x, @3x, etc. trophy images.

The default trophies (fish and sparkly) have been updated with slightly
improved (crisper) visuals and support for Retina images. The sparkly
trophy is now more sparkly, and the fish trophy has a better base and
eye.

Testing Done

Unit tests pass.

Tested that trophies are still properly being calculated and assigned.

Tested that unregistered trophies previously assigned to a review request
no longer breaks the trophy banner.

Tested that multiple trophies renders correctly.

Tested that new-style and legacy trophy subclasses work.

Tested the new trophy images on Retina and non-Retina displays.

Files

Issues

Description	From	Last Updated
Typo in description: "ALl methods"	david	Aug. 18, 2016, 3:06 a.m.
In your description, "presentational" isn't a word.	david	Aug. 18, 2016, 3:06 a.m.
'six' imported but unused	reviewbot	Aug. 12, 2016, 11:39 p.m.
'logging' imported but unused	reviewbot	Aug. 12, 2016, 11:39 p.m.
undefined name 'AlreadyRegisteredError'	reviewbot	Aug. 12, 2016, 11:39 p.m.
undefined name 'six'	reviewbot	Aug. 12, 2016, 11:39 p.m.
undefined name 'six'	reviewbot	Aug. 12, 2016, 11:39 p.m.
Since there are so many items here, can we put one per line?	david	Aug. 18, 2016, 2:53 a.m.
Don't need the "like" in here. Trophies are achievements.	david	Aug. 18, 2016, 2:53 a.m.
perhaps "registered trophy type"?	david	Aug. 18, 2016, 2:53 a.m.
Can you clarify in this comment what type this should be? It's not clear from the field name or description.	david	Aug. 18, 2016, 2:54 a.m.
We don't need the .yay class anymore, right?	david	Aug. 18, 2016, 3:05 a.m.

Tool: Pyflakes
Processed Files:
    reviewboard/accounts/managers.py
    reviewboard/reviews/templatetags/reviewtags.py
    reviewboard/accounts/models.py
    reviewboard/accounts/trophies.py

Ignored Files:
    reviewboard/static/rb/images/trophies/sparkly@2x.png
    reviewboard/static/rb/images/trophies/fish@2x.png
    reviewboard/static/rb/images/trophies/fish.png
    reviewboard/static/rb/css/ui/boxes.less
    reviewboard/static/rb/images/trophy.png
    reviewboard/static/rb/images/fish-trophy.png
    reviewboard/templates/reviews/trophy_box.html
    reviewboard/static/rb/images/trophies/sparkly.png



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/accounts/managers.py
    reviewboard/reviews/templatetags/reviewtags.py
    reviewboard/accounts/models.py
    reviewboard/accounts/trophies.py

Ignored Files:
    reviewboard/static/rb/images/trophies/sparkly@2x.png
    reviewboard/static/rb/images/trophies/fish@2x.png
    reviewboard/static/rb/images/trophies/fish.png
    reviewboard/static/rb/css/ui/boxes.less
    reviewboard/static/rb/images/trophy.png
    reviewboard/static/rb/images/fish-trophy.png
    reviewboard/templates/reviews/trophy_box.html
    reviewboard/static/rb/images/trophies/sparkly.png

reviewboard/accounts/managers.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'six' imported but unused
```
reviewboard/accounts/trophies.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'logging' imported but unused
```
reviewboard/accounts/trophies.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 undefined name 'AlreadyRegisteredError'
```
reviewboard/accounts/trophies.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 undefined name 'six'
```
reviewboard/accounts/trophies.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 undefined name 'six'
```

Change Summary:

Fixed some import issues.

Commit:

33c11793849709cd453a0c629a6937aef4882c3f

b7875faf88397e524aa8901d32632a28a38cfd83

Diff:

Revision 2 (+409 -100)

Show changes

	reviewboard/accounts/managers.py
	reviewboard/accounts/models.py
	reviewboard/accounts/trophies.py
	reviewboard/reviews/templatetags/reviewtags.py
	reviewboard/static/rb/css/ui/boxes.less
	reviewboard/static/rb/images/fish-trophy.png
	reviewboard/static/rb/images/trophy.png
	reviewboard/static/rb/images/trophies/fish.png
	4 more

Tool: Pyflakes
Processed Files:
    reviewboard/accounts/managers.py
    reviewboard/reviews/templatetags/reviewtags.py
    reviewboard/accounts/models.py
    reviewboard/accounts/trophies.py

Ignored Files:
    reviewboard/static/rb/images/trophies/sparkly@2x.png
    reviewboard/static/rb/images/trophies/fish@2x.png
    reviewboard/static/rb/images/trophies/fish.png
    reviewboard/static/rb/css/ui/boxes.less
    reviewboard/static/rb/images/trophy.png
    reviewboard/static/rb/images/fish-trophy.png
    reviewboard/templates/reviews/trophy_box.html
    reviewboard/static/rb/images/trophies/sparkly.png



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/accounts/managers.py
    reviewboard/reviews/templatetags/reviewtags.py
    reviewboard/accounts/models.py
    reviewboard/accounts/trophies.py

Ignored Files:
    reviewboard/static/rb/images/trophies/sparkly@2x.png
    reviewboard/static/rb/images/trophies/fish@2x.png
    reviewboard/static/rb/images/trophies/fish.png
    reviewboard/static/rb/css/ui/boxes.less
    reviewboard/static/rb/images/trophy.png
    reviewboard/static/rb/images/fish-trophy.png
    reviewboard/templates/reviews/trophy_box.html
    reviewboard/static/rb/images/trophies/sparkly.png

The issue has been resolved. Show all issues
```
Typo in description: "ALl methods"
```

The issue has been dropped. Show all issues

In your description, "presentational" isn't a word.

chipx86 Aug. 18, 2016, 3:05 a.m.

Sure it is.

http://www.dictionary.com/browse/presentational

reviewboard/accounts/trophies.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Since there are so many items here, can we put one per line?
```
reviewboard/accounts/trophies.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Don't need the "like" in here. Trophies are achievements.
```
reviewboard/accounts/trophies.py (Diff revision 2)
The issue has been resolved. Show all issues
```
perhaps "registered trophy type"?
```

reviewboard/accounts/trophies.py (Diff revision 2)

The issue has been resolved. Show all issues

Can you clarify in this comment what type this should be? It's not clear from the field name or description.

reviewboard/static/rb/css/ui/boxes.less (Diff revision 2)
The issue has been resolved. Show all issues
```
We don't need the .yay class anymore, right?
```
1. chipx86 Aug. 18, 2016, 3:05 a.m.
  Probably not. Wasn't sure if anything else used it.

Change Summary:


Fixed a typo in the description.
Split up a bulky list of imports to be one per line.
Clarified some docs.
Removed the old yay CSS class.

Description:

		This change makes a small handful of improvements to our trophies,
		covering the implementation of `TrophyType` subclasses, the presentation
		of trophies, and the look of our standard trophies.

		On an implementation level, `TrophyType` subclasses no longer need to
		override `__init__`, as the metadata for the trophy is now stored in class
		attributes. Older subclasses can install provide these through the
		constructor, but will get deprecation warnings.

		Trophy registration is now based on a standard registry. The existing
		methods remain for compatibility, but will show deprecation warnings.

~		ALl methods and attributes are now properly documented as well.
	~	All methods and attributes are now properly documented as well.

		On a presentational level, rendering problems with showing multiple
		trophies or when there's an unknown trophy have been fixed. Trophies are
		also no longer bound to a specific width and height (previously, this
		would result in some weird problems with the positioning of the trophy).

		Trophies can also provide multiple images for different resolution
		levels, allowing for @2x, @3x, etc. trophy images.

		The default trophies (fish and sparkly) have been updated with slightly
		improved (crisper) visuals and support for Retina images. The sparkly
		trophy is now more sparkly, and the fish trophy has a better base and
		eye.

Commit:

b7875faf88397e524aa8901d32632a28a38cfd83

99fa7161de422749226c4eaa4a864fc3c3a6bc4d

Diff:

Revision 3 (+412 -104)

Show changes

	reviewboard/accounts/managers.py
	reviewboard/accounts/models.py
	reviewboard/accounts/trophies.py
	reviewboard/reviews/templatetags/reviewtags.py
	reviewboard/static/rb/css/ie_hacks.css
	reviewboard/static/rb/css/ui/boxes.less
	reviewboard/static/rb/images/fish-trophy.png
	reviewboard/static/rb/images/trophy.png
	5 more

Tool: Pyflakes
Processed Files:
    reviewboard/accounts/managers.py
    reviewboard/reviews/templatetags/reviewtags.py
    reviewboard/accounts/models.py
    reviewboard/accounts/trophies.py

Ignored Files:
    reviewboard/static/rb/images/trophies/sparkly@2x.png
    reviewboard/static/rb/images/trophies/fish@2x.png
    reviewboard/static/rb/css/ie_hacks.css
    reviewboard/static/rb/images/trophies/fish.png
    reviewboard/static/rb/css/ui/boxes.less
    reviewboard/static/rb/images/trophy.png
    reviewboard/static/rb/images/fish-trophy.png
    reviewboard/templates/reviews/trophy_box.html
    reviewboard/static/rb/images/trophies/sparkly.png



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/accounts/managers.py
    reviewboard/reviews/templatetags/reviewtags.py
    reviewboard/accounts/models.py
    reviewboard/accounts/trophies.py

Ignored Files:
    reviewboard/static/rb/images/trophies/sparkly@2x.png
    reviewboard/static/rb/images/trophies/fish@2x.png
    reviewboard/static/rb/css/ie_hacks.css
    reviewboard/static/rb/images/trophies/fish.png
    reviewboard/static/rb/css/ui/boxes.less
    reviewboard/static/rb/images/trophy.png
    reviewboard/static/rb/images/fish-trophy.png
    reviewboard/templates/reviews/trophy_box.html
    reviewboard/static/rb/images/trophies/sparkly.png

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-3.0.x (613b09f)