-
Notifications
You must be signed in to change notification settings - Fork 86
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 user profile feature #1923
Add user profile feature #1923
Conversation
179a88f
to
e100b0b
Compare
On the frontend, the user profile sidebar panel simply shows the current user's name and profile photo. Clicking on the profile photo brings up a context menu that shows the profile and profile actions (only 'edit profile' for now). Clicking the 'edit profile' action changes the context menu view such that you can edit the profile photo. This commit also adds user profile photos to channel messages. A jdenticon is showed in place of user profile photo if it doesn't exist. On the backend, there is a new shared orbitdb user profiles key/value store. The key/value store maps user public keys with profiles. It uses a custom index so that we can filter invalid entries thoroughly. In state-manager, a new saveUserProfile saga is added which signs the profile data and sends it to the backend.
e100b0b
to
ecb8227
Compare
})} | ||
</List> | ||
) | ||
} |
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've separated these out to make the context menu more general purpose. Not all context menu views will have item lists.
packages/desktop/src/renderer/sagas/notifications/notifications.click.test.ts
Outdated
Show resolved
Hide resolved
packages/state-manager/src/sagas/publicChannels/publicChannels.selectors.ts
Outdated
Show resolved
Hide resolved
console.log('Saving user profile', userProfile) | ||
|
||
yield* apply(socket, socket.emit, applyEmitParams(SocketActionTypes.SAVE_USER_PROFILE, userProfile)) | ||
} |
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.
Moving most of this logic to the backend in a future PR will make it easier to test and review.
Now using the same version as backend which also fixes our common JS issues for now.
packages/state-manager/src/sagas/users/userProfile/saveUserProfile.saga.ts
Outdated
Show resolved
Hide resolved
export function* saveUserProfileSaga(socket: Socket, action: PayloadAction<{ photo?: File }>): Generator { | ||
const identity = yield* select(identitySelectors.currentIdentity) | ||
|
||
if (!identity?.userCsr || !action.payload.photo) { |
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'm not sure if this is the case so don't bother if it's not important in this context but if you want to make sure user is a part of a community you should not only check the presence of csr, but also a replication of the community's metadata. On current develop there's a "communityMembership" selector in the identity structure
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, I'm just doing some validation here.
packages/state-manager/src/sagas/users/userProfile/saveUserProfile.saga.ts
Show resolved
Hide resolved
I wouldn't be myself if I didn't ask: what about mobile? |
I think we should make two separate issues for mobile. One would be to display avatars if they exist. The other would be to add and update them. On mobile the minimum viable feature for the latter requires quite a bit more work. We need to implement our design for picking photos, and I think we need compression too: a mobile user will never know how to resize a photo manually on their phone, or if they do we shouldn't ask them to. It's already a stretch on desktop but on mobile an image size limit would feel too broken to be releasable, in my view. The other reason to not go farther now is that this issue was not top of backlog and was chosen as an onboarding task that would give Lucas a good tour of how data moves through the app. I'd rather have Lucas move on to more fundamental stuff on mobile like iOS push notifications experiments. |
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.
This makes sense to me and I think I understood the important stuff about how it works. Great work!
I don't think any of my comments required specific changes, but I'm interested in your responses to the questions I raised.
Sorry about the large PR, I can split it up if desired.
This PR adds a user profile feature for desktop.
On the frontend, the user profile sidebar panel simply shows the current user's
name and profile photo. Clicking on the profile photo brings up a context menu
that shows the profile and profile actions (only 'edit profile' for now).
Clicking the 'edit profile' action changes the context menu view such that you
can edit the profile photo. This commit also adds user profile photos to channel
messages. A jdenticon is showed in place of user profile photo if it doesn't
exist.
On the backend, there is a new shared orbitdb user profiles key/value store. The
key/value store maps user public keys with profiles. It uses a custom index so
that we can filter invalid entries thoroughly. In state-manager, a new
saveUserProfile saga is added which signs the profile data and sends it to the
backend.
Relevant chromatic stories:
Solution for #615
Pull Request Checklist
(Optional) Mobile checklist
Please ensure you completed the following checks if you did any changes to the mobile package: