Skip to content

Commit

Permalink
Enable list save button when a collection is selected. (#27)
Browse files Browse the repository at this point in the history 
Previously, the save button was only enabled when a book was added. Now, the save button is enabled when either a collection is selected, or a book is added.
  • Loading branch information
@ray-lee
ray-lee authored Jun 15, 2022
1 parent 31fb540 commit b430b15
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 58 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
## Changelog

### v0.0.10

#### Fixed

- When editing lists, the save button is now correctly enabled when a collection is selected, even if no books have been added.

### v0.0.9

#### Updated
Expand Down
49 changes: 22 additions & 27 deletions src/components/CustomListEditor.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export default class CustomListEditor extends React.Component<
const hasChanges = this.hasChanges();
// The "save this list" button should be disabled if there are no changes
// or if the list's title is empty.
const disableSave = this.isTitleOrEntriesEmpty() || !hasChanges;
const disableSave = !hasChanges || !this.isValid();
return (
<div className="custom-list-editor">
<div className="custom-list-editor-header">
Expand Down Expand Up @@ -222,22 +222,17 @@ export default class CustomListEditor extends React.Component<
}
}

isTitleOrEntriesEmpty(): boolean {
// Checks if the list is in a saveable state (aka, has a title and books).
if (
(this.props.list?.title || this.state.title) &&
this.state.title !== "list title" &&
this.state.entries.length
) {
return false;
} else {
return (
!this.state.title ||
this.state.title === "list title" ||
this.state.title === "" ||
this.state.entries.length === 0
);
}
/**
* Determines if the list being edited is valid. The list is considered valid if:
* - The title is not empty
* - At least one collection is selected, or at least one book has been added
*
* @returns true if the list is valid, false otherwise
*/
isValid(): boolean {
const { collections, entries, title } = this.state;

return !!(title && (collections?.length || entries?.length));
}

hasChanges(): boolean {
Expand All @@ -246,7 +241,7 @@ export default class CustomListEditor extends React.Component<
let entriesChanged =
this.props.list &&
!!this.props.list.books &&
this.props.list.books.length !== this.state.entries.length;
this.props.list.books.length !== this.state.entries?.length;
// If the current list is new then this.props.list will be undefined, but
// the state for the entries or title can be populated so there's a need to check.
if (!this.props.list) {
Expand All @@ -269,14 +264,14 @@ export default class CustomListEditor extends React.Component<
let collectionsChanged = false;
if (
this.props.listCollections &&
this.props.listCollections.length !== this.state.collections.length
this.props.listCollections.length !== this.state.collections?.length
) {
collectionsChanged = true;
} else {
const propsIds = (this.props.listCollections || [])
.map((collection) => collection.id)
.sort();
const stateIds = this.state.collections
const stateIds = (this.state.collections || [])
.map((collection) => collection.id)
.sort();
for (let i = 0; i < propsIds.length; i++) {
Expand Down Expand Up @@ -306,13 +301,13 @@ export default class CustomListEditor extends React.Component<
});
}

hasCollection(collection: AdminCollectionData) {
for (const stateCollection of this.state.collections) {
if (stateCollection.id === collection.id) {
return true;
}
}
return false;
hasCollection(collection: AdminCollectionData): boolean {
const { collections } = this.state;

return !!(
collections &&
collections.find((candidate) => candidate.id === collection.id)
);
}

changeCollection(collection: AdminCollectionData) {
Expand Down
167 changes: 136 additions & 31 deletions src/components/__tests__/CustomListEditor-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ describe("CustomListEditor", () => {
getEntriesStub.restore();
});

it("shouldn't allow you to save unless the list has a title and entries", () => {
it("enables the save button when the list has changes and is valid", () => {
wrapper = mount(
<CustomListEditor
library={library}
Expand All @@ -263,21 +263,65 @@ describe("CustomListEditor", () => {
{ context: fullContext, childContextTypes }
);

let saveButton = wrapper.find(".save-or-cancel-list").find(Button).at(0);
expect(saveButton.props().disabled).to.equal(true);
wrapper.instance().hasChanges = () => true;
wrapper.instance().isValid = () => true;

wrapper.setState({ title: "new list title" });
saveButton = wrapper.find(".save-or-cancel-list").find(Button).at(0);
wrapper.setProps({});

const saveButton = wrapper.find(".save-or-cancel-list").find(Button).at(0);
expect(saveButton.props().disabled).to.equal(false);
});

it("disables the save button when the list does not have changes", () => {
wrapper = mount(
<CustomListEditor
library={library}
languages={languages}
searchResults={searchResults}
editCustomList={editCustomList}
search={search}
loadMoreSearchResults={loadMoreSearchResults}
loadMoreEntries={loadMoreEntries}
isFetchingMoreSearchResults={false}
isFetchingMoreCustomListEntries={false}
entryPoints={entryPoints}
/>,
{ context: fullContext, childContextTypes }
);

wrapper.instance().hasChanges = () => false;
wrapper.instance().isValid = () => true;

wrapper.setProps({});

const saveButton = wrapper.find(".save-or-cancel-list").find(Button).at(0);
expect(saveButton.props().disabled).to.equal(true);
});

wrapper.setState({
title: "new list title",
entries: [{ id: "1234", title: "a", authors: [] }],
});
saveButton = wrapper.find(".save-or-cancel-list").find(Button).at(0);
it("disables the save button when the list is invalid", () => {
wrapper = mount(
<CustomListEditor
library={library}
languages={languages}
searchResults={searchResults}
editCustomList={editCustomList}
search={search}
loadMoreSearchResults={loadMoreSearchResults}
loadMoreEntries={loadMoreEntries}
isFetchingMoreSearchResults={false}
isFetchingMoreCustomListEntries={false}
entryPoints={entryPoints}
/>,
{ context: fullContext, childContextTypes }
);

expect(saveButton.props().disabled).to.equal(false);
wrapper.instance().hasChanges = () => true;
wrapper.instance().isValid = () => false;

wrapper.setProps({});

const saveButton = wrapper.find(".save-or-cancel-list").find(Button).at(0);
expect(saveButton.props().disabled).to.equal(true);
});

it("navigates to edit page after a new list is created", async () => {
Expand Down Expand Up @@ -726,25 +770,86 @@ describe("CustomListEditor", () => {
expect(hasChanges).to.equal(true);
});
});
it("should know whether the list title is blank, or there are no entries", () => {
// There is a list property with a title and entries
expect(wrapper.instance().isTitleOrEntriesEmpty()).to.be.false;
wrapper.setProps({ list: null });
wrapper.setState({ entries: [] });
wrapper.setState({ title: null });
// New list, no title
expect(wrapper.instance().isTitleOrEntriesEmpty()).to.be.true;
// New list, title is still just the placeholder
wrapper.setState({ title: "list title" });
expect(wrapper.instance().isTitleOrEntriesEmpty()).to.be.true;
// New list, placeholder has been deleted.
wrapper.setState({ title: "" });
expect(wrapper.instance().isTitleOrEntriesEmpty()).to.be.true;
// Adding a title, but no entries...
wrapper.setState({ title: "testing..." });
expect(wrapper.instance().isTitleOrEntriesEmpty()).to.be.true;
// Adding entries...
wrapper.setState({ entries: [{ id: "1234", title: "a", authors: [] }] });
expect(wrapper.instance().isTitleOrEntriesEmpty()).to.be.false;

describe("isValid", () => {
it("should return false if the title is blank", () => {
wrapper.setState({
title: "",
entries: [{ id: "1234", title: "a", authors: [] }],
collections: [{ id: 2, name: "collection 2", protocol: "protocol" }],
});

expect(wrapper.instance().isValid()).to.be.false;
});

it("should return false if the title is null", () => {
wrapper.setState({
title: null,
entries: [{ id: "1234", title: "a", authors: [] }],
collections: [{ id: 2, name: "collection 2", protocol: "protocol" }],
});

expect(wrapper.instance().isValid()).to.be.false;
});

it("should return false if the title is undefined", () => {
wrapper.setState({
title: undefined,
entries: [{ id: "1234", title: "a", authors: [] }],
collections: [{ id: 2, name: "collection 2", protocol: "protocol" }],
});

expect(wrapper.instance().isValid()).to.be.false;
});

it("should return false if entries and collections are both null", () => {
wrapper.setState({
title: "List 1",
entries: null,
collections: null,
});

expect(wrapper.instance().isValid()).to.be.false;
});

it("should return false if entries and collections are both undefined", () => {
wrapper.setState({
title: "List 1",
entries: undefined,
collections: undefined,
});

expect(wrapper.instance().isValid()).to.be.false;
});

it("should return false if entries and collections are both empty", () => {
wrapper.setState({
title: "List 1",
entries: [],
collections: [],
});

expect(wrapper.instance().isValid()).to.be.false;
});

it("should return true if title is not blank and entries is not empty", () => {
wrapper.setState({
title: "List 1",
entries: [{ id: "1234", title: "a", authors: [] }],
collections: [],
});

expect(wrapper.instance().isValid()).to.be.true;
});

it("should return true if title is not blank and collections is not empty", () => {
wrapper.setState({
title: "List 1",
entries: [],
collections: [{ id: 2, name: "collection 2", protocol: "protocol" }],
});

expect(wrapper.instance().isValid()).to.be.true;
});
});
});

0 comments on commit b430b15

Please sign in to comment.