Rachael O'James
Rachael O'James

Reputation: 132

Div onClick function doesn't work when clicking in React app

I have a function which creates 2 divs when changing the number of items correspondingly (say if we choose 5 TVs we will get 5 divs for choosing options). They serve to make a choice - only one of two possible options should be chosen for every TV, so when we click on one of them, it should change its border and background color.

Now I want to create a dynamic stylization for these divs: when we click on them, they should get a new class (tv-option-active) to change their styles.

For that purposes I used 2 arrays (classesLess and classesOver), and every time we click on one of divs we should remove a class if it's already applied to the opposite option and push the class to the target element - thus only one of options will have tv-option-active class.

But when I click on a div I do not get anything - when I open the document in the browser and inspect the elements, the elements do not even receive new class on click - however, when I console log the classes variable that should apply to an element, it is the way it should be - "less tv-option-active" or "over tv-option-active". I applied join method to remove the comma.

I checked the name of imported css file and it is ok so the problem is not there, also I applied some rules just to make sure the problem is not there and it worked when it's not dynamic (I mean no clicks are needed).

So my list of reasons causing that trouble seems to be not working. I also tried to reorganize the code in order to not call a function in render return - putting mapping directly to render return, but this also didn't work.

Can anyone give me a hint why it is that?

Here is my code.

import React from 'react'
import { NavLink, withRouter } from 'react-router-dom'
import './TVMonting.css'

import PageTitle from '../../PageTitle/PageTitle'

class TVMontingRender extends React.Component {
    state = {
        tvData: {
            tvs: 1,
            under: 0, 
            over: 0,
        },
    }


    render() {
        let classesLess = ['less']
        let classesOver = ['over']

        const tvHandlers = {
            tvs: {
                decrease: () => {
                    if (this.state.tvData.tvs > 1) {
                        let tvData = {
                            ...this.state.tvData,
                        }
                        tvData.tvs -= 1

                        this.setState({ tvData })
                    }
                },
                increase: () => {
                    if (this.state.tvData.tvs < 5) {
                        let tvData = {
                            ...this.state.tvData,
                        }

                        tvData.tvs += 1
                        this.setState({ tvData })
                    }
                },
            },
        }

        const createDivs = () => {
            const divsNumber = this.state.tvData.tvs

            let divsArray = []
            for (let i = 0; i < divsNumber; i++) {
                divsArray.push(i)
            }

            return divsArray.map((i) => {
                return (
                    <React.Fragment key={i}>
                        <div
                            className={classesLess.join(
                                ' '
                            )}
                            onClick={() => {
                                const idx = classesOver.indexOf(
                                    'tv-option-active'
                                )
                                if (idx !== -1) {
                                    classesLess.splice(
                                        idx,
                                        1
                                    )
                                }
                                classesLess.push(
                                    'tv-option-active'
                                )
                            }}
                        >
                            Under 65
                        </div>
                        <div
                            className={classesOver.join(
                                ' '
                            )}
                            onClick={() => {
                                const idx = classesLess.indexOf(
                                    'tv-option-active'
                                )
                                if (idx !== -1) {
                                    classesOver.splice(
                                        idx,
                                        1
                                    )
                                }

                                classesOver.push(
                                    'tv-option-active'
                                )

                                // classesOver.join(' ')
                            }}
                        >
                            Over 65
                        </div>
                    </React.Fragment>
                )
            })
        }

        return (
            <div>

                <button onClick={tvHandlers.tvs.decrease}>
                    -
                </button>

                {this.state.tvData.tvs === 1 ? (
                    <h1> {this.state.tvData.tvs} TV </h1>
                ) : (
                    <h1> {this.state.tvData.tvs} TVs </h1>
                )}

                <button onClick={tvHandlers.tvs.increase}>
                    +
                </button>

                {createDivs()}

             </div>
        )
    }
}

export default withRouter(TVMontingRender)

CSS file is very simple - it just adds a border.

P.S. I know that with this architecture when I click on one of the divs all the divs will get tv-option-active class, but for now I just want to make sure that this architecture works - since I'm relatively new in React 🙂Thanks in advance!

Upvotes: 1

Views: 3924

Answers (1)

buzatto
buzatto

Reputation: 10382

Components won't have their lifecycle triggered if you are mutating a variable. You need a state for that purpose, which stores the handled data.

In your case you need some state to say which div has the active class, under or over. You can also abstract each rendered Tv to another Class component. This way you achieve independent elements that control their own class, rather than changing all others.

For that I created a Tv class, where I simplified some of the logic:

class Tv extends React.Component {

  state = {
    activeGroup: null 
  }

  // this will update which group is active
  changeActiveGroup = (activeGroup) => this.setState({activeGroup})

  // activeClass will return 'tv-option-active' if that group is active
  activeClass = (group) => (group === this.state.activeGroup ? 'tv-option-active' : '')

  render () {
    return (
      <React.Fragment>
        <div
          className={`less ${ activeClass('under') }`}
          onClick={() => changeActiveGroup('under')}
        >
            Under 65
        </div>
        <div
          className={`over ${ activeClass('over') }`}
          onClick={() => changeActiveGroup('over')}
        >
            Over 65
        </div>
      </React.Fragment>
    )
  }
}

Your TvMontingRender will be cleaner, also it's better to declare your methods at your class body rather than inside of render function:

class TVMontingRender extends React.Component {
    state = {
        tvData: {
            tvs: 1,
            under: 0, 
            over: 0,
        }
    }

    decreaseTvs = () => {
      if (this.state.tvData.tvs > 1) {
          let tvData = {
              ...this.state.tvData,
          }
          tvData.tvs -= 1

          this.setState({ tvData })
      }
    }

  increaseTvs = () => {
    if (this.state.tvData.tvs < 5) {
        let tvData = {
            ...this.state.tvData,
        }

        tvData.tvs += 1
        this.setState({ tvData })
    }
  }

  createDivs = () => {
    const divsNumber = this.state.tvData.tvs

    let divsArray = []
    for (let i = 0; i < divsNumber; i++) {
        divsArray.push(i)
    }
   
    // it would be better that key would have an unique generated id (you could use uuid lib for that) 
    return divsArray.map((i) => <Tv key={i} />)
  } 

    render() {

        return (
            <div>

                <button onClick={this.decreaseTvs}>
                    -
                </button>

                {this.state.tvData.tvs === 1 ? (
                    <h1> {this.state.tvData.tvs} TV </h1>
                ) : (
                    <h1> {this.state.tvData.tvs} TVs </h1>
                )}

                <button onClick={this.increaseTvs}>
                    +
                </button>

                {this.createDivs()}

             </div>
        )
    }
}

Note: I didn't change the key you are passing to Tv, but when handling an array that you manipulate somehow it's often better to pass an unique id identifier. There are some libs for that like uuid, nanoID.

When handling complex class logic, you may consider using libs like classnames, that would make your life easier.

Upvotes: 1

Related Questions