Page MenuHomePhabricator

Badges don't show in certain Timeline conditions
Closed, ResolvedPublic

Description

Badges don't show it seems when other actions happen in addition to a comment, such as an inline comment in differential or closing a task with a comment action.

Event Timeline

chad created this task.Jul 23 2015, 8:06 PM
chad claimed this task.
chad raised the priority of this task from to Needs Triage.
chad updated the task description. (Show Details)
chad added a project: Badges.
chad moved this task to Backlog on the Badges board.
chad added subscribers: chad, epriestley.

Oh, come on now, You're no fun... Why limit badges? Let users look like russian generals during parade!

(max 2)

I can has config value?

chad added a comment.Jul 24 2015, 10:30 PM

I suppose I could just put a number in a circle if you have more than two, forcing you to hover and see the tokens, but fundamentally the main reason I'm opposed to lots of badges is that it greatly diminishes the value of them. I'd love this to be a system where there are 100's of badges -- but you (or we programmatically) need to pick the best two to follow you everywhere. The rest need to stay at the Pokemon Center.

chad added a comment.Jul 24 2015, 10:36 PM

@epriestley, I've dug around in this, but don't really understand timeline rendering after reading the code for the past 20 minutes. Any easy hints?

Let me see if I can give you a skeleton for it.

chad added a comment.Jul 24 2015, 10:38 PM

PhabricatorApplicationTransaction.php is very scary

chad added a comment.Jul 24 2015, 10:39 PM

I think you're right on needing to order the badges by quality. If you have 5, we'd want to show the rarest two.

chad added a comment.Jul 24 2015, 11:13 PM

omg roll it

chad closed this task as Resolved.Jul 24 2015, 11:17 PM

(This might have some bugs or whatever, idk.)

chad added a comment.Jul 24 2015, 11:19 PM

they certainly contribute to the flavor of the project.

I think they could probably be like 1/2 or 1/3 the size they currently are and still get the point across, maybe.

chad added a comment.Jul 24 2015, 11:20 PM

1/3, really?

chad reopened this task as Open.Jul 25 2015, 4:39 AM

yeah some bugs, idk

chad added a comment.Jul 25 2015, 4:41 AM

(when it compresses multiple actions, i think)

I think they could probably be like 1/2 or 1/3 the size they currently are and still get the point across, maybe.

I agree. Seems too big. personal opinion

chad renamed this task from Add badges to timeline to Badges don't show in certain Timeline conditions.Jul 28 2015, 10:50 PM
chad updated the task description. (Show Details)
chad moved this task from Backlog to v0 on the Badges board.Jul 28 2015, 10:52 PM
revi added a subscriber: revi.Oct 4 2015, 2:47 PM
chad triaged this task as Low priority.Dec 10 2015, 9:24 PM

Here's the remaining bug: when a transaction group mixes a comment and another action, badges are not shown, even though we render the profile image in large size and have space to show them. Here's a screenshot of the bug from this task:

Badges should be shown in the red circle, but are not. I believe the problem is that the first group is "reopen + comment" (does not work), while the second group is comment only (works properly).

D13708 is the change which implemented the original behavior.

I haven't looked at it, so I don't know how easy this is to fix.

chad moved this task from v0 to v0 on the Badges board.Feb 6 2016, 5:54 AM
chad edited projects, added Badges (v0); removed Badges.
epriestley reassigned this task from chad to lpriestley.Mar 25 2016, 7:09 PM

Take a look at this and see how far you can get? It might be really easy but it might also be very involved.

lpriestley closed this task as Resolved.Mar 30 2016, 6:36 PM

This is done. Minor followup: T10698.

urzds added a subscriber: urzds.Jul 12 2017, 11:14 AM