search

estroBiologist / pluralchum

PluralKit integration for BetterDiscord
MIT License
42 stars 12 forks source link

add user profile support and bump to 2.3.0 #44

Open 0xeac7 opened 4 months ago

0xeac7 commented 4 months ago

please suggest any additions to the code or things to be changed. thank you!

ariagivens commented 4 months ago

Thanks for the PR! I haven't had a chance to look at the code yet but heres some notes after clicking around:

Bugs

When you generate a profile popup by clicking on a name instead a profile picture, it doesn't go away when you click away. Sometimes profiles show up totally blank. Console gets spammed with:

[BetterDiscord] [Patcher] Could not fire instead callback of getUser for Pluralchum TypeError: Cannot read properties of undefined (reading 'avatarDecorationData')

Changes

A few changes to make it look and work more like profile popup in regular discord.

Take the link underline color from the username color instead of white. The display name part of the popup (big one at top) should look how it does in the server. The username part (the smaller, lower part) was the base account name.

Nice to haves but maybe hard to implement??

Showing base account roles? Either getting rid of the "note" section or using the note from the base account?

0xeac7 commented 4 months ago

done! feel free to review this when you have time

estroBiologist commented 4 months ago

small bug i noticed on the latest commit - clicking on a PK username can leave its message stuck in the hovered state, even if you close the profile and try hovering over the message again. re-opening the profile seems to fix it

also noticed on an older commit that Discord tends to hang when the window switches from unfocused to focused, with a significant amount of time being spent in a function called by the Components.Popout.prototype patch. this issue seems to get worse the longer Discord is running - not sure if this has been fixed, but so far the latest commit seems more responsive at least

image

0xeac7 commented 4 months ago

Fixed everything except the colour bit - it doesn't seem to break the logic, just defaults it for profiles. Should be good to go anyway :)

commented from other account. mb

0xeac7 commented 4 months ago

also - since this is a big update, would it be better to bump the version higher?

ariagivens commented 4 months ago

also - since this is a big update, would it be better to bump the version higher?

we roughly follow semver