diff --git a/CHANGELOG.md b/CHANGELOG.md index d6e39c99b..edd2af9d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/components/CustomListEditor.tsx b/src/components/CustomListEditor.tsx index 32cf50be1..3e524bcc5 100644 --- a/src/components/CustomListEditor.tsx +++ b/src/components/CustomListEditor.tsx @@ -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 (
@@ -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 { @@ -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) { @@ -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++) { @@ -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) { diff --git a/src/components/__tests__/CustomListEditor-test.tsx b/src/components/__tests__/CustomListEditor-test.tsx index 59f2113a5..ff23f7631 100644 --- a/src/components/__tests__/CustomListEditor-test.tsx +++ b/src/components/__tests__/CustomListEditor-test.tsx @@ -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( { { 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( + , + { 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( + , + { 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 () => { @@ -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; + }); }); });