-
-
Notifications
You must be signed in to change notification settings - Fork 12.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking โSign up for GitHubโ, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add avatar of server database version upload to S3, removing SSO dependency for avatar management #7152
base: main
Are you sure you want to change the base?
Conversation
๐ @RICHQAQ Thank you for raising your pull request and contributing to our Community |
@RICHQAQ is attempting to deploy a commit to the LobeChat Desktop Team on Vercel. A member of the Team first needs to authorize it. |
I believe that some information, such as avatars, doesnโt need to be tightly synced with the SSO provider. Just like many apps (e.g., WeChat, Google login), the username and avatar can be modified directly by the user without affecting the authentication service. This separation allows for a more flexible and user-friendly experience. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! There is some part need to be improved~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Loading shouldn't use message loading. need another uploading ui
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been changed to use the Spin
to indicate the uploading process. Is this approach feasible?
|
||
const lobeUser = { | ||
// ๅคดๅไฝฟ็จ่ฎพ็ฝฎ็๏ผ่ไธๆฏไป next-auth ไธญ่ทๅ | ||
avatar: userAvatar || '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should use userAvatar first and then fallback to next-auth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I previously used: avatar: userAvatar || nextUser.image || ''
.
However, I noticed a user experience issue: when refreshing the page after updating the avatar, since nextUser.image still maintains the old value, there would be a momentary flash - showing the old avatar briefly before switching to the updated one.
I considered two solutions:
- Use userAvatar directly while skipping nextUser.image, since during initial page load,
useInitUserState
reads the latest avatar from the database and saves it to the Store, which basically ensures userAvatar's availability. I've tested this and found no bugs. - Synchronously update nextUser.image when the avatar is updated.
I chose the first, as nextUser.image should preserve the original data from the identity provider, while custom uploaded avatars are provided through userAvatar.
If the reviewer thinks another implementation approach would be better, or has a better solution, I'm happy to make the corresponding adjustments.
๐ป ๅๆด็ฑปๅ | Change Type
๐ ๅๆด่ฏดๆ | Description of Change
This PR introduces a feature that allows direct upload of avatars to S3 for the server database version. Previously, avatar changes had to be made through the SSO provider, which was a cumbersome process. With the new implementation, users can now upload avatars directly to S3 and store the image URL in the database, removing the SSO dependency for avatar management. Other data will still remain synchronized with the SSO provider.
๐ ่กฅๅ ไฟกๆฏ | Additional Information
No other functionality was affected. Only the avatar-related mechanism has been modified, making the process more efficient and user-friendly.