Tobi
Tobi

Reputation: 43

I create a useState of a single object containing the states i want to manage, is that a bad practice?

I believe my problem is originating from the disableButton function, but I can't wrap my head around what is wrong

if the sum of all the values of each content is equal to zero purchasable should be false meaning the button should be disabled but even when I add contents to the cart it is still disabled and sometimes a specific number of values needs to be gotten to make button not-disabled and then it goes back to disabled afterward any help would be appreciated :)

const App = props => {
  const [items, setItems] = useState({
    contents: {
      bacon: 0,
      cheese: 0,
    },
    purchasable: false, /* this is to set the disabled state of the purchase button to true || false */
    // ...
  }) /* i set my states this way to avoid calling const multiple times is that a bad practice ? */

  /* function gets an outdated content so it takes a parameter from the 2 buttons that are going to be updating the state the most adding and removing functions  */

  const disableButton = (updatedContent) => {
    const contentsSum = Object.values(updatedContent).reduce((acc, curr) => (acc + curr), 0)
    setItems(items => ({
      ...items,
      purchasable: contentSum > 0 /* idk if something is wrong with this "check" as button gets disabled even when cart isnt empty */
    }))
  }
  
  /* this is to add contents to a cart */
  const addContent(type) => {
    updatedContents = {...items.contents}
    updatedContents[type] = contentCount + 1
    setItems(items => ({
      ...items,
      content: updatedContents
    }))
    disableButton(updatedContent)
  }

  /* this removes content from the cart as long as it isnt empty */
  const removeContent(type) => {
    const contentCount = items.contents[type]
    if(contentCount < 0) return
    updatedContents = {...items.contents}
    updatedContents[type] = contentCount - 1
    setItems(items => ({
      ...items,
      content: updatedContents
    }))
    disableButton(updatedContents)
  }
  
  return (
    <div>
      <ContentControls purchasable={items.purchasable} /> /* the purchasable state is passes to a "place order" that gets disabled based on items.purchasable's state */
    </div>
  )
};

Upvotes: 3

Views: 78

Answers (2)

Mason Clark
Mason Clark

Reputation: 221

Since I can't comment yet, I'll have to put it here. One, it seems like contentCount is not defined in addContent. So you're setting

updatedContent[type] = undefined + 1

Which I believe is NaN. This could cause problems in your disableButton function. Also, in your removeContent function you're checking if the value is less than zero, which I believe should be checking if it's less than or equal to zero, or greater than zero.

Also as a previous commenter said, you could add a useEffect as items as a dependency, and update the purchasable state there. Purchasable should be its own state as shown in the reference provided by another commentor, which would reduce the complexity and less spread syntax.

Edit: So here is a basic jfiddle outlining what you're trying to do. https://jsfiddle.net/9v3cesfg/

As you can see I separated the items from the purchasable logic. There is probably a better way to create the items lists, but this is okay for now. Instead of calling a function every time the list is changed as you did in your example, the logic is now in a useEffect function that runs every time the itemlist changes. You can see how simple the logic is for determining if the items are purchasable since you separated the data.

A couple notes on this. The cloning is done in the add and remove count because of how objects are defined and passed in javascript. When you set a const or variable equal to previously defined object, you're actually setting the const to a reference of that object. So if you mutate a property on const, than you are mutating the same property on the original const. Spread syntax copies the object/array instead of referencing it.

Another note is about the key for the div inside the map function being set to Math.random(). I would never do this in an actual application. React uses key to manage it's DOM events, so they should always be unique with no possibility of duplicate keys. Instead I would use a library such as UUID which systemically generates unique keys

Upvotes: 1

Tobi
Tobi

Reputation: 43

Alright, thank y'all... took Mason's advice and used useEffect then realized the problem was with the way the html input property disabled gets its value. Button gets disabled b'cos contentSum is indeed greater than 0 which is true, sotherefore sets disabled state for the "place order" button to true when cart isn't emty. thanks Mason :) And noticed my code had some missing bits, sorry about that

const App = props => {
  const [items, setItems] = useState({
    contents: {
      bacon: 0,
      cheese: 0,
    },
    purchasable: false, /* this is to set the disabled state of the purchase button to 
    true or false [edit: this is to be set to true by default not false] */
    // ...
  }) /* i set my states this way to avoid calling const multiple times,
    is that a bad practice ? */

  /* function gets an outdated content so it takes a parameter from the 2 functions
    that are going to be updating the state || contents state the most the 
    adding and removing functions  */

  const disableButton = (updatedContent) => {
    const contentsSum = Object.values(updatedContent).reduce((acc, curr) => (acc + curr), 0)
    setItems(items => ({
      ...items,
      purchasable: contentSum > 0 /* idk if something is wrong with this "check" 
    as button gets disabled even when cart isnt empty 
    [edit: nothing was wrong with the check, it was doing its job. i just needed to invert 
    the boolean result to suit the value that the disabled state would accept 
    i.e. !contentSum > 0] */
    }))
  }
  
  /* this is to add contents to a cart */
  const addContent(type) => {
    const oldContentCount = items.contents[type]
    const updatedContents = {...items.contents}
    updatedContents[type] = oldContentCount + 1
    setItems(items => ({
      ...items,
      content: updatedContents
    }))
    disableButton(updatedContent)
  }

  /* this removes content from the cart as long as it isnt empty */
  const removeContent(type) => {
    const oldContentCount = items.contents[type]
    const updatedContents = {...items.contents}
    updatedContents[type] = oldContentCount > 0 ? oldContentCount - 1 : 0
    setItems(items => ({
      ...items,
      content: updatedContents
    }))
    disableButton(updatedContents)
  }
  
  return (
    <Aux>
      /* the purchasable state is passes to a "place order" button that gets disabled 
    based on items.purchasable's state */
      <ContentControls purchasable={items.purchasable} />
    </Aux>
  )
};

Upvotes: 0

Related Questions