Skip to content

[Fix] Prevent empty name category from being added #3761

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
May 25, 2024
Merged

[Fix] Prevent empty name category from being added #3761

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1de161d
Fix: Prevent empty name category from being added
DenysMb May 18, 2024
9d53ebd
Refactor: Trim input to prevent category name with blank spaces only
DenysMb May 20, 2024
f64d412
Refactor: Remove unnecessary optional chaining operator
DenysMb May 21, 2024
ed010a5
Test: Add E2E scenario for disable "Add" category button when empty n…
DenysMb May 22, 2024
ec25f1a
Refactor: Change test scenario comment
DenysMb May 25, 2024
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
17 changes: 16 additions & 1 deletion e2e/categories.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ electronTest('categories', async (app) => {

await expect(dialog.getByText('No categories yet.')).toBeInViewport()

// add button should be disabled without a name
await expect(dialog.getByTitle('Add', { exact: true })).toBeDisabled()

// add new category
await dialog.getByPlaceholder('Add new category').fill('Great games')
await dialog.getByTitle('Add', { exact: true }).click()
Expand All @@ -23,8 +26,20 @@ electronTest('categories', async (app) => {
dialog.locator('span', { hasText: 'Great games' })
).toBeInViewport()

// rename category
// add button should be disable if trying to rename category to empty name
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here you are testing that the button is disabled when the name is unchanged, the comment doesn't match that

the other 2 expects are for the blank tests

I can merge this after that's fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay in fixing this.
These previous days were very busy.
I fixed this now. Where it says "empty" should be "same", I didn't realize that "words swap".

await dialog.getByTitle('Rename "Great games"').click()
await dialog.getByLabel('Rename "Great games"').fill('Great games')
await expect(dialog.getByTitle('Add', { exact: true })).toBeDisabled()

// add button should be disabled if trying to rename category to empty string
await dialog.getByLabel('Rename "Great games"').fill('')
await expect(dialog.getByTitle('Add', { exact: true })).toBeDisabled()

// add button should be disabled if trying to rename category to empty spaces
await dialog.getByLabel('Rename "Great games"').fill(' ')
await expect(dialog.getByTitle('Add', { exact: true })).toBeDisabled()

// rename category
await dialog.getByLabel('Rename "Great games"').fill('Amazing games')
await dialog
.getByTitle('Confirm rename of "Great games" as "Amazing games"')
Expand Down
6 changes: 6 additions & 0 deletions src/frontend/screens/Library/components/CategoriesManager/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ function CategoryItem({
const [renameMode, setRenameMode] = useState(false)
const [newName, setNewName] = useState(name)
const [removeMode, setRemoveMode] = useState(false)
const isNewNameEmptyOrEqualsOldName =
newName.trim() === '' || newName === name

const rename = () => {
renameFunction(name, newName)
Expand Down Expand Up @@ -57,6 +59,7 @@ function CategoryItem({
'Confirm rename of "{{oldName}}" as "{{newName}}"',
{ oldName: name, newName }
)}
disabled={isNewNameEmptyOrEqualsOldName}
>
<FontAwesomeIcon icon={faCheck} />
</button>
Expand Down Expand Up @@ -157,6 +160,8 @@ function CategoriesManager() {

const [newCategoryName, setNewCategoryName] = useState('')

const isCategoryNameEmpty = newCategoryName.trim() === ''

const removeCategory = (cat: string) => {
customCategories.removeCategory(cat)
}
Expand Down Expand Up @@ -206,6 +211,7 @@ function CategoriesManager() {
className="button"
onClick={() => addCategory()}
title={t('categories-manager.add', 'Add')}
disabled={isCategoryNameEmpty}
>
<FontAwesomeIcon icon={faAdd} />
</button>
Expand Down
Loading