Skip to content

Commit

Permalink
✨ Better warning for long logo data uri (#440)
Browse files Browse the repository at this point in the history
* πŸ₯… Feat: handle long svg data uri

feat: handle long svg data uri

* 🎨 Refactor: svg uri error handling

refactor: svg uri error handling

* βœ… Test: playwright test for long svg data uri

test: playwright test for long svg data uri

* 🎨 Refactor: input component and tests

refactor: input component and tests

* βœ… Update logo input logic and tests

---------

Co-authored-by: Wei He <github@weispot.com>
  • Loading branch information
spudoodle and wei authored Dec 17, 2024
1 parent b97240d commit e7f708e
Show file tree
Hide file tree
Showing 10 changed files with 174 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/lucky-terms-protect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"socialify": minor
---

Added error handling for long svg data uri input, also added jest unit test cases for this.
33 changes: 31 additions & 2 deletions .playwright/mainUIConsistency.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ test.describe('Socialify UI:', () => {
// Wait for the page to load/hydrate completely.
await page.waitForLoadState('networkidle', customPageLoadTimeout)

// To maintain consistency, de-select the 'Stars' checkbox,
// and selects the 'Description' checkbox.
await page.click('input[name="stargazers"]')
await page.click('input[name="description"]')

Expand All @@ -73,4 +71,35 @@ test.describe('Socialify UI:', () => {
const toastImage = await page.screenshot(customScreenshotOptions)
expect(toastImage).toMatchSnapshot(customDiffPixelRatio)
})

test('shows error when svg data uri input length exceeds the limit', async ({
page,
}: { page: Page }): Promise<void> => {
await page.goto(repoPreviewURL, customPageLoadTimeout)

// Wait for the page to load/hydrate completely.
await page.waitForLoadState('networkidle', customPageLoadTimeout)

await page
.locator('[data-input-key="logo"] input')
.fill(
'data:image/svg+xml,%3Csvg%20width%3D%22800px%22%20height%3D%22800px%22%20viewBox%3D%220%200%201024%201024%22%20class%3D%22icon%22%20%20version%3D%221.1%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%3E%3Cpath%20d%3D%22M373.2%20600.3h278.7v8H373.2z%22%20fill%3D%22%23999999%22%20%2F%3E%3Cpath%20d%3D%22M512.6%20948.3h-9.8c-58.7%200-106.7-48-106.7-106.7v-212h259v176.2c0%2078.4-64.2%20142.5-142.5%20142.5z%22%20fill%3D%22%23F9C0C0%22%20%2F%3E%3Cpath%20d%3D%22M511.7%20958.8c-40.7%200-79-15.9-108-44.9s-44.9-67.3-44.9-108V209.2h-32.2c-11.4%200-20.7-9.3-20.7-20.7v-17.6c0-11.4%209.3-20.7%2020.7-20.7h370.1c11.4%200%2020.7%209.3%2020.7%2020.7v17.6c0%2011.4-9.3%2020.7-20.7%2020.7h-32.2v596.7c0%2040.7-15.9%2079-44.9%20108-28.9%2028.9-67.2%2044.9-107.9%2044.9zM326.6%20165.1c-3.2%200-5.7%202.6-5.7%205.7v17.6c0%203.2%202.6%205.7%205.7%205.7h47.2v611.7c0%2036.7%2014.4%2071.3%2040.5%2097.4%2026.1%2026.1%2060.7%2040.5%2097.4%2040.5s71.3-14.4%2097.4-40.5c26.1-26.1%2040.5-60.7%2040.5-97.4V194.2h47.2c3.2%200%205.7-2.6%205.7-5.7v-17.6c0-3.2-2.6-5.7-5.7-5.7l-370.2-0.1z%22%20fill%3D%22%23999999%22%20%2F%3E%3Cpath%20d%3D%22M373.2%20193.8h50.7v8h-50.7zM466.8%20193.8h185.1v8H466.8z%22%20fill%3D%22%23999999%22%20%2F%3E%3Cpath%20d%3D%22M535.7%20558.5c-14.1%200-25.5-11.4-25.5-25.5s11.4-25.5%2025.5-25.5%2025.5%2011.4%2025.5%2025.5c0%2014-11.4%2025.5-25.5%2025.5z%20m0-43c-9.6%200-17.5%207.8-17.5%2017.5%200%209.6%207.8%2017.5%2017.5%2017.5s17.5-7.8%2017.5-17.5-7.9-17.5-17.5-17.5zM458.1%20417.6c-21.3%200-38.6-17.3-38.6-38.6s17.3-38.6%2038.6-38.6%2038.6%2017.3%2038.6%2038.6-17.3%2038.6-38.6%2038.6z%20m0-69.2c-16.9%200-30.6%2013.7-30.6%2030.6s13.7%2030.6%2030.6%2030.6%2030.6-13.7%2030.6-30.6-13.7-30.6-30.6-30.6zM566.7%20107.3c-11.4%200-20.7-9.3-20.7-20.7s9.3-20.7%2020.7-20.7%2020.7%209.3%2020.7%2020.7-9.2%2020.7-20.7%2020.7z%20m0-33.4c-7%200-12.7%205.7-12.7%2012.7s5.7%2012.7%2012.7%2012.7%2012.7-5.7%2012.7-12.7-5.7-12.7-12.7-12.7zM540.5%20299.5c-16.7%200-30.3-13.6-30.3-30.3s13.6-30.3%2030.3-30.3%2030.3%2013.6%2030.3%2030.3-13.6%2030.3-30.3%2030.3z%20m0-52.6c-12.3%200-22.3%2010-22.3%2022.3s10%2022.3%2022.3%2022.3%2022.3-10%2022.3-22.3-10-22.3-22.3-22.3z%22%20fill%3D%22%23CE0202%22%20%2F%3E%3C%2Fsvg%3E'
)

const errorMessage = await page
.locator('[data-input-key="logo"] .label-text-alt.text-red-400')
.textContent()
expect(errorMessage).toBe(
'URI is too long, please use an SVG image URL instead.'
)

await page.click('input[name="stargazers"]')
await page.click('input[name="description"]')

// Wait for the component transition/animation to finish completely.
await page.waitForTimeout(1000)

const image = await page.screenshot(customScreenshotOptions)
expect(image).toMatchSnapshot(customDiffPixelRatio)
})
})
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 0 additions & 2 deletions .playwright/simpleUserStory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ test.describe('A simple user story:', () => {
await page.waitForLoadState('networkidle', customTimeout)
await expect(page).toHaveURL(expectedConfigURL)

// To maintain consistency, de-select the 'Stars' checkbox,
// and selects the 'Description' checkbox.
await page.click('input[name="stargazers"]')
await page.click('input[name="description"]')

Expand Down
3 changes: 2 additions & 1 deletion src/components/configuration/config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import InputWrapper from '@/src/components/configuration/inputWrapper'
import SelectWrapper from '@/src/components/configuration/selectWrapper'
import TextAreaWrapper from '@/src/components/configuration/textAreaWrapper'
import ConfigContext from '@/src/contexts/ConfigContext'
import LogoInput from './logoInput'

type ConfigProp = {
repository: RepoQueryResponse['repository']
Expand Down Expand Up @@ -162,7 +163,7 @@ const Config = ({ repository }: ConfigProp) => {
value={config.pattern}
handleChange={handleChange}
/>
<InputWrapper
<LogoInput
title="SVG Logo"
alt="Image url or data uri"
keyName="logo"
Expand Down
66 changes: 66 additions & 0 deletions src/components/configuration/inputWrapper.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { fireEvent, render, screen } from '@testing-library/react'
import '@testing-library/jest-dom'
import InputWrapper, { type InputProps } from './inputWrapper'

describe('Renders input wrapper correctly', () => {
const mockHandleChange = jest.fn()

const baseProps: InputProps = {
title: 'Test Input Label',
keyName: 'name',
value: '',
placeholder: 'Test Placeholder',
handleChange: mockHandleChange,
}

test('renders the label correctly', () => {
render(<InputWrapper {...baseProps} />)

const labelElement = screen.getByText('Test Input Label')
expect(labelElement).toBeInTheDocument()
expect(labelElement).toHaveClass('label-text')
})

test('renders the alt label correctly', () => {
render(<InputWrapper {...baseProps} alt="Test Alt Label" />)

const altLabelElement = screen.getByText('Test Alt Label')
expect(altLabelElement).toBeInTheDocument()
expect(altLabelElement).toHaveClass('label-text-alt')
})

test('renders the placeholder correctly', () => {
render(<InputWrapper {...baseProps} />)

const inputElement = screen.getByPlaceholderText('Test Placeholder')
expect(inputElement).toBeInTheDocument()
})

test('renders disabled input correctly', () => {
render(<InputWrapper {...baseProps} disabled />)

const inputElement = screen.getByPlaceholderText('Test Placeholder')
expect(inputElement).toBeDisabled()
})

test('renders input changes correctly', () => {
render(<InputWrapper {...baseProps} />)

const inputElement = screen.getByPlaceholderText('Test Placeholder')
fireEvent.change(inputElement, { target: { value: 'Test Input' } })
expect(mockHandleChange).toHaveBeenCalledWith(
{ val: 'Test Input', required: true },
'name'
)
})

test('renders error correctly', () => {
render(<InputWrapper {...baseProps} error="Test Error" />)

const inputElement = screen.getByPlaceholderText('Test Placeholder')
expect(inputElement).toHaveClass('input-error')
const errorElement = screen.getByText('Test Error')
expect(errorElement).toBeInTheDocument()
expect(errorElement).toHaveClass('text-red-400')
})
})
16 changes: 13 additions & 3 deletions src/components/configuration/inputWrapper.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import type ConfigType from '@/common/types/configType'

type InputProps = {
export type InputProps = {
title: string
alt?: string
keyName: keyof ConfigType
value: string
placeholder: string
disabled?: boolean
handleChange: (value: any, key: keyof ConfigType) => void
error?: string
maxlen?: number
}

const InputWrapper = ({
Expand All @@ -18,23 +20,31 @@ const InputWrapper = ({
placeholder,
disabled,
handleChange,
error,
maxlen,
}: InputProps) => {
return (
<div className="form-control w-full">
<div className="form-control w-full" data-input-key={keyName}>
<label className="label">
<span className="label-text font-semibold">{title}</span>
{alt && <span className="label-text-alt font-semibold">{alt}</span>}
</label>
<input
className="input input-sm input-bordered font-semibold w-full"
className={`input input-sm input-bordered font-semibold w-full ${error ? 'input-error' : ''}`}
type="text"
value={value || ''}
disabled={!!disabled}
placeholder={placeholder}
onChange={(e) => {
handleChange({ val: e.target.value, required: true }, keyName)
}}
maxLength={maxlen}
/>
{error && (
<div className="label">
<span className="label-text-alt text-red-400">{error}</span>
</div>
)}
</div>
)
}
Expand Down
40 changes: 40 additions & 0 deletions src/components/configuration/logoInput.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { render, screen } from '@testing-library/react'
import '@testing-library/jest-dom'
import type { InputProps } from './inputWrapper'
import LogoInput from './logoInput'

describe('Renders logo input correctly', () => {
const mockHandleChange = jest.fn()

const baseProps: InputProps = {
title: 'Test Input Label',
keyName: 'logo',
value: '',
placeholder: 'Test Placeholder',
handleChange: mockHandleChange,
error: 'URI is too long, please use an SVG image URL instead.',
maxlen: 1601,
}

test('renders error message when uri is greater than 1600 characters', () => {
render(<LogoInput {...baseProps} value={'a'.repeat(1601)} />)

const inputElement = screen.getByPlaceholderText('Test Placeholder')
expect(inputElement).toHaveClass('input-error')
const errorElement = screen.getByText(
'URI is too long, please use an SVG image URL instead.'
)
expect(errorElement).toBeInTheDocument()
})

test('does not renders error message when uri is less than 1601 characters', () => {
render(<LogoInput {...baseProps} value={'a'.repeat(1600)} />)

const inputElement = screen.getByPlaceholderText('Test Placeholder')
expect(inputElement).not.toHaveClass('input-error')
const errorElement = screen.queryByText(
'URI is too long, please use an SVG image URL instead.'
)
expect(errorElement).not.toBeInTheDocument()
})
})
17 changes: 17 additions & 0 deletions src/components/configuration/logoInput.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import InputWrapper, { type InputProps } from './inputWrapper'

const LogoInput = (props: InputProps) => {
return (
<InputWrapper
{...props}
maxlen={1601}
error={
props.value?.length >= 1601
? 'URI is too long, please use an SVG image URL instead.'
: undefined
}
/>
)
}

export default LogoInput

0 comments on commit e7f708e

Please sign in to comment.