Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Static checks (lint / typecheck / build) on every push, and a GitHub Pages
# deploy on master. Cypress e2e lives in its own workflow so the two run in
# parallel.
# Static checks (lint / typecheck / unit tests / build) on every push, and a
# GitHub Pages deploy on master. Cypress e2e lives in its own workflow so the
# two run in parallel.

name: Node.js CI

Expand All @@ -23,6 +23,7 @@ jobs:
- run: yarn install --frozen-lockfile
- run: yarn lint
- run: yarn typecheck
- run: yarn test
- run: yarn build
- name: prepare deploy
if: github.ref == 'refs/heads/master'
Expand Down
57 changes: 57 additions & 0 deletions cypress/e2e/validation_spec.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { interceptStarredSinglePage } from '../support/intercepts'

/**
* Which names are valid is settled by the unit tests in
* `src/utils/validateUsername.test.ts`. These tests only ask whether the
* validator is wired into the form at all: a name GitHub could never own
* must not reach the network, and the user must be told why.
*/
describe('Username validation', function () {
beforeEach(() => {
interceptStarredSinglePage()
cy.visit('/')
})

it('should not call the API when the username could not exist', () => {
cy.get('.search-input').type('oct--ocat')
cy.get('.search-button').click()

cy.get('.search-error')
.should('have.attr', 'role', 'alert')
.and('contain', 'letters, numbers, and single hyphens')

// The app never leaves the welcome screen, and no request is made.
// (`.card` is not usable here: the welcome screen renders sample cards.)
cy.get('.welcome').should('exist')
cy.get('@starred.all').should('have.length', 0)
})

it('should mark the input invalid for assistive technology', () => {
cy.get('.search-input').type('my_name')
cy.get('.search-button').click()

cy.get('.search-input').should('have.attr', 'aria-invalid', 'true')
})

it('should search normally once the name is corrected', () => {
cy.get('.search-input').type('my_name')
cy.get('.search-button').click()
cy.get('.search-error').should('exist')

cy.get('.search-input').clear().type('octocat')
cy.get('.search-button').click()

cy.get('.search-error').should('not.exist')
cy.get('.search-input').should('not.have.attr', 'aria-invalid', 'true')
cy.get('.card').should('have.length', 3)
cy.get('@starred.all').should('have.length', 1)
})

it('should leave valid names untouched', () => {
cy.get('.search-input').type('octo-cat')
cy.get('.search-button').click()

cy.get('.search-error').should('not.exist')
cy.get('.card').should('have.length', 3)
})
})
8 changes: 7 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,17 @@
"react": "^19.2.8",
"react-dom": "^19.2.8"
},
"resolutions": {
"vite": "^7.3.2"
},
"scripts": {
"cy:open": "cypress open",
"cy:run": "cypress run",
"cy:run:chrome": "cypress run --browser chrome",
"cy:run:firefox": "cypress run --browser firefox",
"start": "vite",
"test": "vitest run",
"test:watch": "vitest",
"lint": "eslint src",
"typecheck": "tsc --noEmit",
"build": "tsc --noEmit && vite build"
Expand All @@ -34,6 +39,7 @@
"typescript": "^6.0.3",
"typescript-eslint": "^8.66.0",
"vite": "^7.3.2",
"vite-plugin-compression": "^0.5.1"
"vite-plugin-compression": "^0.5.1",
"vitest": "^4.1.10"
}
}
30 changes: 26 additions & 4 deletions src/components/SearchBar.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { useState } from 'react'
import { isValidUsername, USERNAME_ERROR_MESSAGE } from '../utils/validateUsername'
import '../css/SearchBar.css'

interface Props {
Expand All @@ -8,11 +9,22 @@ interface Props {

export const SearchBar: React.FC<Props> = (props) => {
const [name, setName] = useState('')
// Only set on submit: warning while the user is still typing would flag
// every name as broken before it is finished.
const [invalid, setInvalid] = useState(false)

const onFormSubmit = (event: React.FormEvent<HTMLFormElement>) => {
event.preventDefault()
if (name.trim()) {
props.onSubmit(name.trim())
const trimmed = name.trim()
if (!trimmed) {
return
}
if (!isValidUsername(trimmed)) {
setInvalid(true)
return
}
setInvalid(false)
props.onSubmit(trimmed)
}

return (
Expand All @@ -27,12 +39,17 @@ export const SearchBar: React.FC<Props> = (props) => {
type="text"
autoFocus
value={name}
onChange={(e) => setName(e.target.value)}
onChange={(e) => {
setName(e.target.value)
// Drop a stale warning as soon as the name changes.
setInvalid(false)
}}
readOnly={props.readOnly}
placeholder="Enter GitHub username"
className="search-input"
aria-labelledby="search-heading"
aria-describedby="search-hint"
aria-describedby={invalid ? 'search-error search-hint' : 'search-hint'}
aria-invalid={invalid ? 'true' : undefined}
aria-required="true"
/>
</label>
Expand All @@ -45,6 +62,11 @@ export const SearchBar: React.FC<Props> = (props) => {
{props.readOnly ? 'Searching...' : 'Search'}
</button>
</div>
{invalid && (
<p id="search-error" className="search-error" role="alert">
{USERNAME_ERROR_MESSAGE}
</p>
)}
<p id="search-hint" className="search-hint">Press Enter or click Search to find starred repositories</p>
</div>
</form>
Expand Down
8 changes: 8 additions & 0 deletions src/css/SearchBar.css
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,14 @@
transform: none;
}

.search-error {
margin-top: 0.75rem;
font-size: 0.875rem;
color: #d32f2f;
text-align: center;
line-height: 1.5;
}

.search-hint {
margin-top: 0.75rem;
font-size: 0.875rem;
Expand Down
38 changes: 38 additions & 0 deletions src/utils/validateUsername.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { describe, it, expect } from 'vitest'
import { isValidUsername, USERNAME_MAX_LENGTH } from './validateUsername'

describe('isValidUsername', () => {
describe('accepts names GitHub could own', () => {
it.each([
['a plain name', 'octocat'],
['a single character', 'a'],
['digits only', '123'],
['mixed case', 'Hyuraku'],
['letters and digits', 'user123'],
['a single hyphen inside', 'my-name'],
['hyphens spread apart', 'a-b-c-d'],
['the maximum length', 'a'.repeat(USERNAME_MAX_LENGTH)],
])('%s: %s', (_case, name) => {
expect(isValidUsername(name)).toBe(true)
})
})

describe('rejects names GitHub could not own', () => {
it.each([
['empty input', ''],
['one over the maximum', 'a'.repeat(USERNAME_MAX_LENGTH + 1)],
['a leading hyphen', '-octocat'],
['a trailing hyphen', 'octocat-'],
['two hyphens in a row', 'oct--ocat'],
['only a hyphen', '-'],
['an underscore', 'my_name'],
['a dot', 'my.name'],
['an inner space', 'my name'],
['an at sign', 'user@example'],
['a slash', 'octocat/repo'],
['non-ASCII letters', 'ユーザー'],
])('%s: %s', (_case, name) => {
expect(isValidUsername(name)).toBe(false)
})
})
})
30 changes: 30 additions & 0 deletions src/utils/validateUsername.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* The longest username GitHub allows.
* Kept next to the validator so the test and the UI copy cannot drift apart.
*/
export const USERNAME_MAX_LENGTH = 39

/**
* Shown when the input does not describe a username GitHub could own.
* A single sentence covers every rule, so the user is not walked through
* the rules one rejection at a time.
*/
export const USERNAME_ERROR_MESSAGE =
'Usernames may only contain letters, numbers, and single hyphens (max 39 characters).'

/**
* Answers whether GitHub could own this username, so an input that can only
* ever 404 never reaches the API.
*
* GitHub's rules: letters, digits and hyphens only; no leading or trailing
* hyphen; no two hyphens in a row; 1 to 39 characters.
*
* @param name the raw input, already trimmed by the caller
*/
export const isValidUsername = (name: string): boolean => {
return (
name.length > 0 &&
name.length <= USERNAME_MAX_LENGTH &&
/^[A-Za-z0-9]+(?:-[A-Za-z0-9]+)*$/.test(name)
)
}
Loading