Monorepo for Tangled โ€” https://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.

0
by camsmith.dev 0 comments
expand 1 commit
appview/{db,pages,models}: show tooltips for user handles when hovering on reactions
sign up or login to add to the discussion
1
by camsmith.dev 6 comments
expand 1 commit
appview/{db,pages,models}: show tooltips for user handles when hovering on reactions

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

sign up or login to add to the discussion
2
by camsmith.dev 1 comment
expand 1 commit
appview/{db,pages,models}: show tooltips for user handles when hovering on reactions

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

pull request successfully merged
sign up or login to add to the discussion
Labels

None yet.

assignee

None yet.

Participants 3
AT URI
at://did:plc:5pio6u2p274ls56agcm7nz3q/sh.tangled.repo.pull/3m2izmuycuf22