Skip to content
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

Create Avatar component #4

Merged
merged 7 commits into from
Mar 15, 2023
Merged

Create Avatar component #4

merged 7 commits into from
Mar 15, 2023

Conversation

germain-gg
Copy link
Contributor

@germain-gg germain-gg commented Mar 11, 2023

For element-hq/compound#117
For element-hq/compound#137

Screenshot 2023-03-11 at 13 52 57

Screenshot 2023-03-11 at 13 52 26

Screenshot 2023-03-11 at 13 52 23

Screenshot 2023-03-11 at 13 52 17

@janogarcia would you be able to provide me with 8 color codes for the username mapping?

@germain-gg germain-gg requested a review from a team as a code owner March 11, 2023 11:21
@germain-gg germain-gg requested a review from justjanne March 14, 2023 10:09
src/components/Avatar/Avatar.tsx Show resolved Hide resolved
Comment on lines +19 to +43
const imgCache = {
__cache: new Map<string, Promise<unknown> | boolean>(),
read(src: string): boolean {
if (!this.__cache.has(src)) {
// Create a cache entry with a promise to notify the image is still being loadedd
this.__cache.set(
src,
new Promise((resolve) => {
const img = new Image();
img.onload = () => {
this.__cache.set(src, true);
resolve(this.__cache.get(src));
};
img.src = src;
}).then(() => {
this.__cache.set(src, true);
})
);
}
if (this.__cache.get(src) instanceof Promise) {
throw this.__cache.get(src);
}
return this.__cache.get(src) as boolean;
},
};
Copy link

@justjanne justjanne Mar 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this considers an image, once it has loaded, cached forever, it might give false positives for images that have been evicted by the browser. On the other hand it might lead to a flash of the fallback image if the browser still has an image cached from a previous session. It also doesn't handle errors while loading images correctly yet.

Fine for now, but in the future, it might be ideal if we implement this in a better way.

const MAX = 8;
// Sum up the values of all the char codes in the string
const charCodeSum = id.split("").reduce((sum, char) => {
return sum + char.charCodeAt(0);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that for names containing unicode characters outside the basic multilingual plane this will use the surrogate pairs. It relies on the specific UCS-2 16-bit encoding common to JS/Java/Windows/Qt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed with Janne, most of our codebase will use utf-8 and that shouldn't present a great threat.
There's also a chance that those colors would go away altogether.

We're punting this decision to a later stage

@germain-gg germain-gg merged commit 688d0f0 into main Mar 15, 2023
@germain-gg germain-gg deleted the gsouquet/avatar branch March 15, 2023 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants