CODEWITHSUNDEEP
reactjsreact-router
bobby
bobby

Reputation: 183

Onclick on a navlink issue in react.js state

import React, { Component } from 'react';
import { NavLink } from 'react-router-dom';

class Home extends Component {
    constructor(props) {
        super(props);
        this.state = {
            id: null,
            newpage: '',
        };

        this.refresh = this.refresh.bind(this);
    }

    refresh() {
        const geturl = new URLSearchParams(window.location.search);
        const getid = urlParams.get('id');
        this.setState({ id: getid });
    }

    componentDidMount() {
        // axios commands to get navigation bar setting it newpage state
        this.refresh();
    }

    render() {
        const id = this.state.id;
        const newpage = this.state.newpage.map(o => (
            <NavLink to={'/Home?id=' + o.id} onClick={this.refresh}>
                {o.name}
            </NavLink>
        ));

        return (
            <div>
                {id != null ? 
                    <NavLink to={'/Home?id=' + id}>
                        <button>Change page</button>
                    </NavLink>
             : null}
            </div>
        );
    }
}

export default Home;

Basically I have a navigation bar that sets the page

from /Home to Home?id=1, Home?id=2, etc when you click on it. It's a navlink with a onclick on it so we can refresh the id for the button "change page". I want the id to update everytime I click on this navlink.

The issue is I have with this code is that I have to click it twice for it to update the onpage button to be the correct id assuming im not already on the correct id link. How would I achieve this ?

Upvotes: 4

Views: 32115

Answers (4)

Melchia
Melchia

Reputation: 24234

ComponenentDidMount is called after render. This is the order of calls of react Hooks: componentWillMount --> render --> componentDidMount

So you either move your refresh call this.refresh(); to your render method. Or else you can use componentWillMount instead of componentDidMount

   componentWillMount() {
        // axios commands to get navigation bar setting it newpage state
        this.refresh();
    }

Upvotes: 1

Smerk
Smerk

Reputation: 91

There are a few anti-patterns here and I'd be unable to diagnose exactly what was wrong without seeing the project myself but from what you've described below should work. Unless you wanted deep linking in which you'd have to re-add the componentdidMount URL parameters fetch.

import { NavLink } from 'react-router-dom';

class Home extends Component {

  constructor(props) {
    super(props);
    this.state = {
      id: 0, // or whatever you want it to be initially
    };
    this.incrementId = this.incrementId.bind(this);
  }

 incrementId() {
   this.setState({
     id: id++
   })
 }

  render() {
    return (
      <div>
          <NavLink to={"/Home?id=" + this.state.id} onClick={() => this.incrementId()}>
            <Button>Change page</Button>
          </NavLink>
          : null
      </div>
    );
  }
}


export default Home;

I hope this helps at all.

Upvotes: 0

Dan Prince
Dan Prince

Reputation: 29989

@lt1's answer is spot on (and you should mark it as correct), but I'll just modify your example to help illustrate the point.

import React, { Component } from 'react';
import { NavLink } from 'react-router-dom';

class Home extends Component {
    constructor(props) {
        super(props);
        this.state = {
            newpage: '',
        };
    }

    get currentId() {
        const urlParams = new URLSearchParams(window.location.search);
        return urlParams.get('id');
    }

    render() {
        const id = this.currentId;

        const newpage = this.state.newpage.map(o => (
            <NavLink to={'/Home?id=' + o.id} onClick={this.refresh}>
                {o.name}
            </NavLink>
        ));

        return (
            <div>
                {id != null ? 
                    <NavLink to={'/Home?id=' + id}>
                        <button>Change page</button>
                    </NavLink>
             : null}
            </div>
        );
    }
}

export default Home;

Upvotes: 1

lt1
lt1

Reputation: 781

if I'm understanding the question correctly, it sounds like the issue boils down to the value of this.state.id not being up to date in your render method.

One way to resolve that would be to just not store that on state at all- because it can already be derived from window.location.search, you can just directly access it from that within the render method (using the first two lines of your refresh method.)

in general, it's best to only store things on state that cannot be derived with some sort of calculation in the render method- otherwise you're essentially duplicating state from somewhere else (in this case, from window.location.search). There are some exceptions to this, but I think it's a good rule of thumb.

Upvotes: 1

Related Questions