Skip to content

Commit

Permalink
Add better fallbacks for missing avatar image and name (#88)
Browse files Browse the repository at this point in the history
* Add better fallbacks for missing avatar image and name

* Add initial fallbacks to first_name and email

* Add tests for fallback behavior

* update vite to latest
  • Loading branch information
franknoirot authored Jan 5, 2024
1 parent f97ea4f commit f647fde
Show file tree
Hide file tree
Showing 8 changed files with 1,475 additions and 156 deletions.
8 changes: 7 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,18 @@
"@playwright/test": "^1.28.1",
"@sveltejs/adapter-auto": "^2.0.0",
"@sveltejs/kit": "^1.20.4",
"@testing-library/jest-dom": "^6.2.0",
"@testing-library/svelte": "^4.0.5",
"@types/testing-library__jest-dom": "^6.0.0",
"@types/three": "^0.157.2",
"@typescript-eslint/eslint-plugin": "^6.0.0",
"@typescript-eslint/parser": "^6.0.0",
"@vitest/ui": "^1.1.2",
"autoprefixer": "^10.4.16",
"eslint": "^8.28.0",
"eslint-config-prettier": "^8.5.0",
"eslint-plugin-svelte": "^2.30.0",
"jsdom": "^23.0.1",
"postcss": "^8.4.31",
"postcss-custom-media": "^10.0.2",
"prettier": "^2.8.0",
Expand All @@ -36,7 +41,8 @@
"typescript": "^5.0.0",
"vite": "^4.4.2",
"vite-plugin-mkcert": "^1.16.0",
"vitest": "^0.32.2"
"vitest": "1.1.3",
"vitest-dom": "^0.1.1"
},
"type": "module",
"dependencies": {
Expand Down
46 changes: 30 additions & 16 deletions src/components/AccountMenu.svelte
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
<script lang="ts">
import type { Models } from '@kittycad/lib'
import Person from 'components/Icons/Person.svelte'
import { paths } from '$lib/paths'
import Person from './Icons/Person.svelte'
export let user: Models['User_type']
let open = false
let displayImage = true
let shouldDisplayImage = Boolean(user?.image && user.image !== '')
let shouldDisplayInitial =
!shouldDisplayImage &&
((user?.name && user.name.length > 0) ||
(user?.first_name && user.first_name.length > 0) ||
(user?.email && user.email.length > 0))
function dismiss(e: KeyboardEvent) {
if (e.key === 'Escape') {
Expand All @@ -16,32 +21,41 @@

<div class={'relative flex justify-center items-center ' + (open ? 'open' : '')}>
<button
class={'toggle border border-solid hover:border-green overflow-hidden bg-currentColor w-8 h-8 md:w-12 md:h-12 ' +
(displayImage ? 'rounded-full' : '')}
class={'toggle grid place-content-center border border-solid hover:border-green overflow-hidden bg-currentColor w-8 h-8 md:w-12 md:h-12 ' +
(shouldDisplayInitial || shouldDisplayImage ? 'rounded-full' : 'rounded')}
on:click={() => {
open = !open
}}
on:keydown={dismiss}
>
{#if displayImage}
<img
src={user.image}
alt="Avatar"
class="object-fill"
referrerpolicy="no-referrer"
on:error={() => {
displayImage = false
}}
<img
src={user.image}
alt="Avatar"
class="object-fill"
style={`display: ${shouldDisplayImage ? 'block' : 'none'}`}
referrerpolicy="no-referrer"
/>
{#if shouldDisplayInitial}
<span
class="w-5 h-5 font-bold text-xl leading-[1] pt-0.5 text-center text-chalkboard-10 dark:text-chalkboard-120"
data-testid="initial">{user.name[0] || user.first_name[0] || user.email[0]}</span
>
{:else if !shouldDisplayImage}
<Person
data-testid="person-icon"
class="w-full text-chalkboard-10 dark:text-chalkboard-120"
/>
{:else}
<Person class="object-fill p-1 block text-chalkboard-10 dark:text-chalkboard-120" />
{/if}
<span class="sr-only">Open menu</span>
</button>
<dialog class="menu">
<menu class="contents">
<div class="p-4 pb-2">
<p class="font-mono">{user?.name || 'Unnamed User'}</p>
<p class="font-mono">
{user?.first_name
? user.first_name + (user.last_name ? user.last_name : '')
: user?.name || 'Unnamed User'}
</p>
<p class="font-mono text-sm text-chalkboard-70 dark:text-chalkboard-40">
{user?.email || '[email protected]'}
</p>
Expand Down
97 changes: 97 additions & 0 deletions src/components/AccountMenu.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import { render } from '@testing-library/svelte'
import AccountMenu from './AccountMenu.svelte'
import type { Models } from '@kittycad/lib'

const FULL_USER: Models['User_type'] = {
id: 'some_id',
name: 'Frank Noirot',
email: '[email protected]',
image:
'https://lh3.googleusercontent.com/a/ACg8ocIiX1-R981041VeC5g5SCFVUzS2rUvkNPSVgD35gSy_lEU=s96-c',
phone: '',
first_name: 'Another',
last_name: 'Alias',
created_at: '1993-09-16T19:33:17.783Z',
updated_at: '2023-10-13T19:33:17.783Z',
company: '',
discord: '',
github: ''
}
const NAME_USER: Models['User_type'] = {
...FULL_USER,
image: ''
}
const FIRST_NAME_USER: Models['User_type'] = {
...NAME_USER,
name: ''
}
const EMAIL_USER: Models['User_type'] = {
...FIRST_NAME_USER,
first_name: ''
}
const FAILED_USER: Models['User_type'] = {
...EMAIL_USER,
email: ''
}

describe('AccountMenu', async () => {
it('avatar image appears if available', async () => {
const component = await render(AccountMenu, {
props: {
user: FULL_USER
}
})

expect(component.getByAltText('Avatar')).toBeVisible()
})

it('fallback initial appears if name is available', async () => {
const component = await render(AccountMenu, {
props: {
user: NAME_USER
}
})

expect(component.getByAltText('Avatar')).not.toBeVisible()
const initial = await component.getByTestId('initial')
expect(initial).toBeVisible()
expect(initial.textContent).toBe(NAME_USER.name[0])
})

it('fallback initial appears if first_name is available', async () => {
const component = await render(AccountMenu, {
props: {
user: FIRST_NAME_USER
}
})

expect(component.getByAltText('Avatar')).not.toBeVisible()
const initial = await component.getByTestId('initial')
expect(initial).toBeVisible()
expect(initial.textContent).toBe(FIRST_NAME_USER.first_name[0])
})

it('fallback initial appears if email is available', async () => {
const component = await render(AccountMenu, {
props: {
user: EMAIL_USER
}
})

expect(component.getByAltText('Avatar')).not.toBeVisible()
const initial = await component.getByTestId('initial')
expect(initial).toBeVisible()
expect(initial.textContent).toBe(FIRST_NAME_USER.email[0])
})

it('user outline appears if all other options fail', async () => {
const component = await render(AccountMenu, {
props: {
user: FAILED_USER
}
})

expect(component.getByAltText('Avatar')).not.toBeVisible()
expect(await component.getByTestId('person-icon')).toBeVisible()
})
})
2 changes: 1 addition & 1 deletion tests/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ import { expect, test } from '@playwright/test'

test('index page has expected h1', async ({ page }) => {
await page.goto('/')
await expect(page.getByRole('heading', { name: 'Welcome to SvelteKit' })).toBeVisible()
await expect(page.getByRole('heading', { name: 'Text-to-CAD' })).toBeVisible()
})
3 changes: 2 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"skipLibCheck": true,
"sourceMap": true,
"strict": true,
"moduleResolution": "Bundler"
"moduleResolution": "Bundler",
"types": ["vitest/globals", "@testing-library/jest-dom"]
}
// Path aliases are handled by https://kit.svelte.dev/docs/configuration#alias
//
Expand Down
6 changes: 5 additions & 1 deletion vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ export default defineConfig(({ mode }) => {
https: mode === 'development'
},
test: {
include: ['src/**/*.{test,spec}.{js,ts}']
globals: true,
include: ['src/**/*.{test,spec}.{js,ts}'],
environment: 'jsdom',
// Extend jest-dom matchers
setupFiles: ['./vitest-setup.ts']
},
ssr: {
noExternal: ['three']
Expand Down
1 change: 1 addition & 0 deletions vitest-setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import 'vitest-dom/extend-expect'
Loading

0 comments on commit f647fde

Please sign in to comment.