Skip to content

Unique issuers should have different colors #659

Closed
wants to merge 4 commits into from

4 participants

@andrewhayward
Mozilla member

Fixing #655 by automatically generating colours for issuers where not provided.

@stenington stenington and 1 other commented on an outdated diff Mar 5, 2013
models/badge.js
var Badge = function (attributes) {
+ if (attributes) {
+ try {
+ var issuer = attributes.body.badge.issuer;
+
+ if (issuer && !issuer.color) {
+ var color = generateColor(attributes.body.badge.issuer.name);
+
+ attributes.body.badge.issuer.color = {
+ value: color,
+ dark: isDarkColor(color)
+ }
+ }
+ } catch () {
@stenington
stenington added a note Mar 5, 2013

Travis is dying here. } catch (ex) { works.

@andrewhayward
Mozilla member
andrewhayward added a note Mar 5, 2013

Hmm. That worked for me locally, after having already trashed Travis. Oh well. Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stenington stenington commented on the diff Mar 5, 2013
models/badge.js
var Badge = function (attributes) {
+ if (attributes) {
+ try {
+ var issuer = attributes.body.badge.issuer;
@stenington
stenington added a note Mar 5, 2013

This is the wrong place to store the color. attributes.body is the metadata in the badge assertion, and I'm worried that modifying it means we'll display those modifications back somewhere else on the site. I guess this issue really requires a DB migration. :/

@andrewhayward
Mozilla member
andrewhayward added a note Mar 5, 2013

Yeah, I couldn't see anything anywhere about individual issuers in the DB, other than here. Figured I'd go out on a limb and do something, on the assumption that it'd probably be wrong.

@stenington
stenington added a note Mar 5, 2013

Yep, didn't occur to me until now that this would require DB changes. Feel free to punt for the time being.

You can stick some code in Badge.prepare.out['issuerColor'] or something similar to create a virtual field of sorts. Once you merge in the latest development updates, you can see the way I did this with imageUrl.

But yes, ideally we'd have an issuer table.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@iamjessklein

I am closing this issue as we are moving forward with a different UI design that matches the branding from Openbadges.org

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.