Monorepo for Tangled tangled.org

show handles of users that reacted to issues and pulls #636

merged opened by camsmith.dev targeting master from camsmith.dev/core: feat/user-reaction-handles

Displays the handles of users that reacted on issues and pulls, in addition to the counts. Closes https://tangled.org/@tangled.org/core/issues/246

There's a couple ways to do it

  1. Doing the db query in 1 roundtrip -- that's what I did here
  2. Doing 2 db queries, 1 for the count and 1 for the usernames

If we prefer (2), or another route, that's fine, I can change it.

Labels

None yet.

assignee

None yet.

Participants 3
AT URI
at://did:plc:5pio6u2p274ls56agcm7nz3q/sh.tangled.repo.pull/3m2izmuycuf22
-12
Interdiff #0 โ†’ #1
-12
appview/db/reaction.go
··· 62 62 return count, nil 63 63 } 64 64 65 - func GetReactionCountMap(e Execer, threadAt syntax.ATURI) (map[models.ReactionKind]int, error) { 66 - countMap := map[models.ReactionKind]int{} 67 - for _, kind := range models.OrderedReactionKinds { 68 - count, err := GetReactionCount(e, threadAt, kind) 69 - if err != nil { 70 - return map[models.ReactionKind]int{}, nil 71 - } 72 - countMap[kind] = count 73 - } 74 - return countMap, nil 75 - } 76 - 77 65 func GetReactionMap(e Execer, threadAt syntax.ATURI) (map[models.ReactionKind]models.ReactionDisplayData, error) { 78 66 const maxUsersPerKind = 20 79 67
appview/issues/issues.go

This file has not been changed.

appview/models/reaction.go

This file has not been changed.

appview/pages/pages.go

This file has not been changed.

appview/pages/templates/repo/fragments/reaction.html

This file has not been changed.

appview/pages/templates/repo/issues/issue.html

This file has not been changed.

appview/pages/templates/repo/pulls/fragments/pullHeader.html

This file has not been changed.

appview/pulls/pulls.go

This file has not been changed.

History

3 rounds 7 comments
sign up or login to add to the discussion
1 commit
expand
appview/{db,pages,models}: show tooltips for user handles when hovering on reactions
expand 1 comment

awesome, thanks for the PR! works like a charm.

pull request successfully merged
1 commit
expand
appview/{db,pages,models}: show tooltips for user handles when hovering on reactions
expand 6 comments

I chose an arbitrary cutoff of 20 usernames to display

I have a different implementation separating username resolving from service layer. I'll open a separate PR soon

Actually nvm. This seems fine.

The benefit of (2) being that if there are a lot of reacts, we aren't querying all of them from the db. so it's probably preferable to make two separate small db calls than 1 unbounded one

thanks for the PR! this works like a charm, couple of nits:

  • can we avoid hardcoding maxUsersPerKind in the db module? we can just return all the users here or accept a limit parameter.
  • instead of a custom tooltip, can we use the title attribute? this has the added benefit of being accessible! something like this might work:
        {{ if gt (length .Users) 0 }}
          title="{{ range $i, $did := .Users }}{{ if ne $i 0 }},{{end}} {{ resolve . }}{{ end }}"
        {{ else }}
          title="{{ .Kind }}"
        {{ end }}
  • lastly, the the repo/fragments/reaction fragment is broken in two scenarios: when it is rendered from either of the endpoints in state/reaction.go, this is because Users is not passed in. the rendering itself is fine, but there is a latent error in rendering the template. hovering over a reaction right after you react will not show the tooltip. a refresh will be necessary.

Thank you for the review @opp.li ! Addressed, I believe

1 commit
expand
appview/{db,pages,models}: show tooltips for user handles when hovering on reactions
expand 0 comments