-
Notifications
You must be signed in to change notification settings - Fork 19
chore(components): update post avatar to handle errors if no gravatar is found #5860
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
base: main
Are you sure you want to change the base?
Changes from all commits
3b58568
baa8a96
0996383
59ab239
1e854ed
b77d958
6155c70
9424f61
3fef2e5
21414d2
b7c2091
e4a0992
b7facdc
b059011
1a24435
ed0b312
49f1f70
b431891
03d002e
6111372
234e09e
935cb25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@swisspost/design-system-components': patch | ||
--- | ||
|
||
Updated `<post-avatar>` to handle errors if no Gravatar is found. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
import { getGravatarUrl } from '../../src/components/post-avatar/avatar-utils'; | ||
|
||
const PAGE_ID = '09aac03d-220e-4885-8fb8-1cfa01add188'; | ||
|
||
describe('Avatar', () => { | ||
|
@@ -40,23 +42,26 @@ describe('Avatar', () => { | |
cy.get('@initials').should('have.text', ''); | ||
}); | ||
|
||
it('should show image, when email with gravatar account is defined', () => { | ||
cy.get('@avatar').invoke('attr', 'email', '[email protected]'); | ||
cy.get('@avatar').should('have.attr', 'email'); | ||
cy.get('@avatar').find('slot img').should('exist'); | ||
cy.get('@avatar').find('.initials').should('not.exist'); | ||
it('should show initials if gravatar does not exist, otherwise show img', () => { | ||
const email = '[email protected]'; | ||
const url = getGravatarUrl(email); | ||
|
||
cy.get('@avatar').invoke('removeAttr', 'email'); | ||
cy.get('@avatar').find('slot img').should('not.exist'); | ||
cy.get('@avatar').find('.initials').should('exist'); | ||
}); | ||
cy.request({ | ||
url, | ||
failOnStatusCode: false, | ||
}).then(response => { | ||
cy.get('@avatar').invoke('attr', 'email', email); | ||
cy.get('@avatar').should('have.attr', 'email'); | ||
cy.get('@avatar').should('have.attr', 'firstname'); | ||
|
||
it('should show initials, when email with no gravatar account is defined', () => { | ||
cy.get('@avatar').invoke('attr', 'email', '[email protected]'); | ||
cy.get('@avatar').should('have.attr', 'email'); | ||
cy.get('@avatar').should('have.attr', 'firstname'); | ||
cy.get('@avatar').find('slot img').should('not.exist'); | ||
cy.get('@avatar').find('.initials').should('exist'); | ||
if (response.status === 200) { | ||
cy.get('@avatar').find('slot img').should('exist'); | ||
cy.get('@avatar').find('.initials').should('not.exist'); | ||
} else { | ||
cy.get('@avatar').find('slot img').should('not.exist'); | ||
cy.get('@avatar').find('.initials').should('exist'); | ||
} | ||
}); | ||
}); | ||
|
||
it('should show image, when slotted image is defined', () => { | ||
|
@@ -66,11 +71,35 @@ describe('Avatar', () => { | |
); | ||
cy.get('@avatar').find('slot img').should('not.exist'); | ||
cy.get('@avatar').find('.initials').should('not.exist'); | ||
}); | ||
|
||
cy.get('@avatar').find('> img').invoke('remove'); | ||
it('should not show image, when slotted image is not defined', () => { | ||
cy.get('@avatar').invoke( | ||
'append', | ||
'<img src="/assets/images/logo-swisspost.svg" alt="Swiss Post Logo" />', | ||
); | ||
|
||
cy.get('@avatar').find('img').invoke('remove'); | ||
cy.get('@avatar').find('img').should('not.exist'); | ||
cy.get('@avatar').find('.initials').should('exist'); | ||
}); | ||
|
||
it('should not show image but fallback to initials, when slotted image is invalid', () => { | ||
cy.get('@avatar').invoke( | ||
'append', | ||
'<img src="/assets/images/invalid-image.svg" alt="Invalid image" />', | ||
); | ||
cy.get('@avatar').find('.initials').should('exist'); | ||
myrta2302 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
|
||
it('should show initials, when image is not visible', () => { | ||
cy.get('@avatar').invoke( | ||
'append', | ||
'<img src="/assets/images/logo-swisspost.svg" alt="Swiss Post Logo" />', | ||
); | ||
cy.get('@avatar').find('img').invoke('css', 'display', 'none'); | ||
cy.get('@avatar').find('.initials').should('be.visible'); | ||
}); | ||
}); | ||
|
||
describe('Accessibility', () => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
// https://docs.gravatar.com/api/avatars/images/ | ||
|
||
const GRAVATAR_DEFAULT = '404'; | ||
const GRAVATAR_RATING = 'g'; | ||
const GRAVATAR_SIZE = 80; | ||
|
||
export function getGravatarUrl(email: string): string { | ||
const hash = cryptify(email.trim().toLowerCase()); | ||
return `https://www.gravatar.com/avatar/${hash}?s=${GRAVATAR_SIZE}&d=${GRAVATAR_DEFAULT}&r=${GRAVATAR_RATING}`; | ||
} | ||
|
||
export async function cryptify(key: string) { | ||
return await crypto.subtle.digest('SHA-256', new TextEncoder().encode(key)).then(buffer => { | ||
return Array.from(new Uint8Array(buffer)) | ||
.map(bytes => bytes.toString(16).padStart(2, '0')) | ||
.join(''); | ||
}); | ||
} | ||
|
||
export const GRAVATAR_BASE_URL = `https://www.gravatar.com/avatar/{email}?s=${GRAVATAR_SIZE}&d=${GRAVATAR_DEFAULT}&r=${GRAVATAR_RATING}`; |
oliverschuerch marked this conversation as resolved.
Show resolved
Hide resolved
myrta2302 marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,7 @@ | ||
import { Component, Element, h, Host, Prop, State, Watch } from '@stencil/core'; | ||
import { version } from '@root/package.json'; | ||
import { checkRequiredAndType, checkEmptyOrPattern, checkEmptyOrType } from '@/utils'; | ||
|
||
// https://docs.gravatar.com/api/avatars/images/ | ||
const GRAVATAR_DEFAULT = '404'; | ||
const GRAVATAR_RATING = 'g'; | ||
const GRAVATAR_SIZE = 80; | ||
|
||
const GRAVATAR_BASE_URL = `https://www.gravatar.com/avatar/{email}?s=${GRAVATAR_SIZE}&d=${GRAVATAR_DEFAULT}&r=${GRAVATAR_RATING}`; | ||
import { GRAVATAR_BASE_URL } from './avatar-utils'; | ||
|
||
const emailPattern = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/; | ||
|
||
|
@@ -27,7 +21,10 @@ enum AvatarType { | |
shadow: true, | ||
}) | ||
export class PostAvatar { | ||
private static INTERNAL_USERID_IMAGE_SRC = 'https://web.post.ch/UserProfileImage/{userid}.png'; | ||
private static readonly INTERNAL_USERID_IMAGE_SRC = | ||
'https://web.post.ch/UserProfileImage/{userid}.png'; | ||
|
||
private slottedImageObserver: MutationObserver; // To watch the slotted image src. | ||
|
||
@Element() host: HTMLPostAvatarElement; | ||
|
||
|
@@ -57,6 +54,9 @@ export class PostAvatar { | |
@State() imageAlt = ''; | ||
@State() initials = ''; | ||
|
||
// To handle email or userid updates and reset the storage item | ||
@State() storageKey: string = ''; | ||
|
||
@Watch('firstname') | ||
validateFirstname() { | ||
checkRequiredAndType(this, 'firstname', 'string'); | ||
|
@@ -68,55 +68,74 @@ export class PostAvatar { | |
} | ||
|
||
@Watch('userid') | ||
validateUserid() { | ||
checkEmptyOrType(this, 'userid', 'string'); | ||
updateUserid() { | ||
this.validateUserId(); | ||
this.getAvatarImage(); | ||
} | ||
|
||
@Watch('email') | ||
validateEmail() { | ||
updateEmail() { | ||
this.validateEmail(); | ||
this.getAvatarImage(); | ||
} | ||
|
||
private validateUserId() { | ||
checkEmptyOrType(this, 'userid', 'string'); | ||
} | ||
|
||
private validateEmail() { | ||
if (this.email) checkEmptyOrPattern(this, 'email', emailPattern); | ||
} | ||
|
||
private async getAvatar() { | ||
if (this.slottedImage !== null) { | ||
this.avatarType = AvatarType.Slotted; | ||
} else { | ||
let imageLoaded = false; | ||
private async getAvatarImage() { | ||
let imageLoaded = false; | ||
this.slottedImage = this.host.querySelector('img'); | ||
const imageUrl = this.slottedImage?.getAttribute('src'); | ||
|
||
if (!imageLoaded && this.userid) | ||
if (!imageUrl) { | ||
if (this.userid) { | ||
imageLoaded = await this.getImageByProp(this.userid, this.fetchImageByUserId.bind(this)); | ||
|
||
if (!imageLoaded && this.email) | ||
} | ||
if (!imageLoaded && emailPattern.exec(this.email ?? '') !== null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be much more readable (imo), and also more consistent with the rest of the code, if you'd write it like so:
While |
||
imageLoaded = await this.getImageByProp(this.email, this.fetchImageByEmail.bind(this)); | ||
|
||
if (!imageLoaded) this.getAvatarByInitials(); | ||
} | ||
if (!imageLoaded) { | ||
this.getAvatarInitials(); | ||
} | ||
} else { | ||
const slottedImageLoaded = await this.getImageByProp( | ||
imageUrl, | ||
this.fetchSlottedImage.bind(this), | ||
); | ||
|
||
if (!slottedImageLoaded) { | ||
this.slottedImage.style.display = 'none'; | ||
this.getAvatarInitials(); | ||
} else { | ||
this.slottedImage.style.display = 'block'; | ||
} | ||
} | ||
} | ||
|
||
private async getImageByProp(prop: string, fetchImage: () => Promise<Response>) { | ||
private async getImageByProp(prop: string, fetchImage: (prop?: string) => Promise<Response>) { | ||
if (!prop) return false; | ||
let imageResponse: Response; | ||
|
||
const imageResponse = (await this.getStorageItem(prop)) ?? { ok: false, url: '' }; | ||
|
||
if (!imageResponse.ok) { | ||
try { | ||
const r = await fetchImage(); | ||
|
||
imageResponse.ok = r.ok; | ||
imageResponse.url = r.url; | ||
|
||
this.imageUrl = imageResponse.url; | ||
this.imageAlt = `${this.firstname} ${this.lastname} avatar`; | ||
this.avatarType = AvatarType.Image; | ||
|
||
this.setStorageItem(this.userid, JSON.stringify(imageResponse)); | ||
} catch (error) { | ||
this.removeStorageItem(prop); | ||
console.info(`Loading avatar by type "${AvatarType.Image}" failed.`); | ||
} | ||
try { | ||
imageResponse = await fetchImage(prop); | ||
} catch (error) { | ||
console.info('Loading avatar image failed.', error); | ||
return false; | ||
} | ||
|
||
return imageResponse.ok; | ||
if (!imageResponse?.ok) { | ||
return false; | ||
} else { | ||
this.imageUrl = imageResponse.url; | ||
this.imageAlt = `${this.firstname} ${this.lastname} avatar`; | ||
this.avatarType = AvatarType.Image; | ||
return true; | ||
} | ||
} | ||
|
||
private async fetchImageByUserId() { | ||
|
@@ -131,7 +150,11 @@ export class PostAvatar { | |
return await fetch(imageUrl); | ||
} | ||
|
||
private getAvatarByInitials() { | ||
private async fetchSlottedImage(imageUrl: string) { | ||
return await fetch(imageUrl, { method: 'HEAD' }); | ||
} | ||
|
||
private getAvatarInitials() { | ||
this.initials = this.getInitials(); | ||
this.avatarType = AvatarType.Initials; | ||
} | ||
|
@@ -147,22 +170,6 @@ export class PostAvatar { | |
.trim(); | ||
} | ||
|
||
private async getStorageItem(keyToken: string) { | ||
const key = await this.cryptify(keyToken); | ||
const value = window?.sessionStorage?.getItem(key); | ||
return value ? JSON.parse(value) : null; | ||
} | ||
|
||
private async setStorageItem(keyToken: string, value: string) { | ||
const key = await this.cryptify(keyToken); | ||
window?.sessionStorage?.setItem(key, value); | ||
} | ||
|
||
private async removeStorageItem(keyToken: string) { | ||
const key = await this.cryptify(keyToken); | ||
window?.sessionStorage?.removeItem(key); | ||
} | ||
|
||
private async cryptify(key: string) { | ||
return await crypto.subtle.digest('SHA-256', new TextEncoder().encode(key)).then(buffer => { | ||
return Array.from(new Uint8Array(buffer)) | ||
|
@@ -171,20 +178,47 @@ export class PostAvatar { | |
}); | ||
} | ||
|
||
private onSlotDefaultChange() { | ||
this.slottedImage = this.host.querySelector('img'); | ||
this.getAvatar(); | ||
private slotChanged() { | ||
const slot = this.host.shadowRoot.querySelector('slot'); | ||
const assignedNodes = slot?.assignedNodes({ flatten: true }) || []; | ||
|
||
assignedNodes.forEach(node => { | ||
if (node.nodeType === Node.ELEMENT_NODE) { | ||
const el = node as Element; | ||
if (el.tagName === 'IMG') { | ||
this.observeImageSrcChanges(el as HTMLImageElement); | ||
} | ||
} | ||
}); | ||
|
||
this.getAvatarImage(); | ||
} | ||
myrta2302 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
componentWillRender() { | ||
this.slottedImage = this.host.querySelector('img'); | ||
this.getAvatar(); | ||
// Observe the Slotted image src attribute and update the image | ||
private observeImageSrcChanges(img: HTMLImageElement) { | ||
if (this.slottedImageObserver) { | ||
this.slottedImageObserver.disconnect(); | ||
} | ||
this.slottedImageObserver = new MutationObserver(mutations => { | ||
mutations.forEach(mutation => { | ||
if (mutation.type === 'attributes' && mutation.attributeName === 'src') { | ||
this.getAvatarImage(); | ||
} | ||
}); | ||
}); | ||
this.slottedImageObserver.observe(img, { attributes: true, attributeFilter: ['src'] }); | ||
} | ||
|
||
connectedCallback() { | ||
//This provides a fallback by showing the initials while the image is still loading or delayed. | ||
this.getAvatarInitials(); | ||
myrta2302 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
this.getAvatarImage(); | ||
} | ||
|
||
componentDidLoad() { | ||
this.validateFirstname(); | ||
this.validateLastname(); | ||
this.validateUserid(); | ||
this.validateUserId(); | ||
this.validateEmail(); | ||
} | ||
|
||
|
@@ -199,10 +233,11 @@ export class PostAvatar { | |
|
||
return ( | ||
<Host data-version={version}> | ||
<slot onSlotchange={this.onSlotDefaultChange.bind(this)}> | ||
{this.avatarType === 'image' && <img src={this.imageUrl} alt={this.imageAlt} />} | ||
{this.avatarType === 'initials' && <div class="initials">{initials}</div>} | ||
</slot> | ||
<span class={this.avatarType === 'slotted' ? '' : 'd-none'}> | ||
<slot onSlotchange={this.slotChanged.bind(this)}></slot> | ||
</span> | ||
{this.avatarType === 'image' && <img src={this.imageUrl} alt={this.imageAlt} />} | ||
{this.avatarType === 'initials' && <div class="initials">{initials}</div>} | ||
</Host> | ||
); | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.