Avatars Stage 1 #378
Avatars Stage 1 #378
Conversation
api/lib/index.js
Outdated
| @@ -14,7 +14,7 @@ const start = async () => { | |||
|
|
|||
| // body parser is deprecated so in Express 4.16+ ( we have 4.17.1) we use | |||
| // these two lines for body parsing | |||
| app.use(express.json()); | |||
| app.use(express.json()); //max size mongodb data | |||
There was a problem hiding this comment.
I think this is probably just a leftover.
|
What do you think about adding the avatar on the user object? My reasoning is that we probably won't need to lookup the avatar separately from the user and with the avatar on the user object queries will be very efficient. |
api/lib/controllers/avatar.js
Outdated
| const initials = subject_username | ||
| .match(/(\b\S)?/g) | ||
| .join("") | ||
| .match(/(^\S|\S$)?/g) | ||
| .join("") | ||
| .toUpperCase(); |
There was a problem hiding this comment.
Since the initials are calculated based on the user.username we shouldn't store them. Otherwise there is the potential for the avatar to deviate from the user object. Instead, we can just perform this operation on the frontend in an avatar component.
There was a problem hiding this comment.
How would they deviate? Well have to run the operation for every avatar object there could be 100
There was a problem hiding this comment.
The deviation could happen when a user's username changes after the avatar has been created. It is a little inconvenient that we will have to perform that operation for all of a groups users but even in that worst case I don't think it would take more then a millisecond. Generally you only want to denormalize like that when the operation is highly expensive because data deviations are dangerous.
There was a problem hiding this comment.
Also, we will probably need to calculate it for non-users anyway so I don't think we can really avoid it by doing it here.
There was a problem hiding this comment.
Okay, do we want users to change usernames though?
There was a problem hiding this comment.
I don't see why we would want to block changing them.
There was a problem hiding this comment.
I don't see why we would want to block changing them.
| @@ -0,0 +1,52 @@ | |||
| import { Modal, Avatar } from "@mui/material"; | |||
There was a problem hiding this comment.
I think that it could be a good idea to create a separate account page at user/[_id] (with largely the same format as this modal). That should make it easier for us to allow group members to look at other user's accounts.
There was a problem hiding this comment.
I tried this initially there was just so much white space I couldn't make it look good. If you think well have enough content to give it its own page we could certainly do it.
There was a problem hiding this comment.
With the groups and integrations related things on the page we should have enough content.
|
@zbarnz Is this ready for review? |
Description
Notion Ticket: https://www.notion.so/thomashudsonnotes/User-icons-bc9aee65f1d24687896a9633cb377b5a
Stage 1 of avatars. This pull request includes user avatars that have a bright easily visible background with username initials.
Next stage will include upload functionality and database google link storage without the frontend
Screenshots of feature
Type of change
How Has This Been Tested?
Created new accounts, signed in and out of account and into other ones.
Checklist: