2
\$\begingroup\$

I'm sorry, maybe this question is really stupid but I have really been thinking a lot about it recently, thinking of which type of <prev | next> web navigation approach is better. FYI the stack is React + Redux.

  1. With using name properties (or also we can use something like custom boolean field forward="ture" and check it)

    HTML:

      <span>
          <button name="prev" onClick={this.onYearChange.bind(this)}>Prev</button>
          <div>{this.state.year}</div>
          <button name="next" onClick={this.onYearChange.bind(this)}>Next</button>
      </span>
    

    JS handler:

     onYearChange(e){
            let dir = (e.target.name === 'prev') ? -1 : 1,
                year = this.props.selectedYear + dir;
    
            this.props.onYearChangeHandler(year);
      }
    
  2. With passing custom property dir with inc/dec value

    HTML:

    <span>
       <button dir={-1} onClick={this.onYearChange.bind(this)}>Prev</button>
       <div>{this.state.year}</div>
       <button  dir={1} onClick={this.onYearChange.bind(this)}>Next</button>
    </span>
    

    JS:

    onYearChange(e){
            let dir = e.target.dir,
                year = this.props.selectedYear + dir;
    
            this.props.onYearChangeHandler(year);
    }
    
  3. Or with two separate binded functions

    HTML:

     <span>
         <button onClick={this.toPrevYear.bind(this)}>Prev</button>
         <div>{this.state.year}</div>
         <button onClick={this.toNextYear.bind(this)}>Next</button>
     </span>
    

    JS:

        toPrevYear(e) {
            this.onYearChange(-1);
        };
    
        toNextYear(e) {
            this.onYearChange(1);
        };
    
        onYearChange(val) {
            let year = this.props.selectedYear + val;
    
            this.props.onYearChangeHandler(year);
        }
    

Or is there some better solution for this scenario?

\$\endgroup\$
2
  • 3
    \$\begingroup\$ Can you make it work like ordinary LINK and A elements using the standard REL="next" etc attributes, so that browsers' usual navigation tools work? \$\endgroup\$ Commented Aug 24, 2017 at 10:15
  • \$\begingroup\$ @TobySpeight oh wellp, never tought about it. never done lot of HTML stuff. but still \$\endgroup\$ Commented Aug 24, 2017 at 10:26

3 Answers 3

2
\$\begingroup\$

Think your worrying yourself about small details here, any advantages of one over the other would be minor at best.

I think out of all 3, the last solution would be my preferred one. It's clean from a code separation POV and does exactly what it says on the tin, also easier to change behaviour without affecting the other should that ever need to happen.

FWIW for performance reasons it's recommended you bind UI event handlers in the constructor e.g.

constructor() {
    this.toPrevYear = this.toPrevYear.bind(this);
}
...
render() {
    return <button onClick={this.toPrevYear}>Prev</button>;
}
\$\endgroup\$
1
  • \$\begingroup\$ oh thanks, i've been thinking about such proper separation stuff. but on other hand what makeds me hesitating that mostly its just simply all about inc/dec some parameter \$\endgroup\$ Commented Aug 24, 2017 at 14:31
2
\$\begingroup\$

About the HTML

As pagination is some form of navigation (i.e., links), you should use a elements instead of button elements.

This also allows you to use the sequential link types next and prev.

<a href="/foo" rel="prev">Previous</a>
<a href="/bar" rel="next">Next</a>
\$\endgroup\$
2
\$\begingroup\$

I agree with James that #3 is better because of the separation aspect.

I admit that I am a novice as far as React goes, having typically worked with other frameworks like jQuery, PrototypeJS, AngularJS, VueJS. The perspective below might not really meld well with traditional React techniques, but Event delegation really helps separate the event logic (JS) from the presentation/markup (HTML).

So consider HTML like this:

<span ref={(span) => this.linksContainer = span}>
    <button name="prev" className="pageLink">Prev</button>
    <div>{this.state.year}</div>
    <button name="next" className="pageLink">Next</button>
  </span>

Notice how the buttons don't need to worry about which function gets called - just that it contains the name (either prev or *next) as well as a class name.

Using that HTML, we could add an event delegate to listen for clicks on the element that contains the page links and look for the class name "pageLink". This is especially helpful when there are numerous elements on the page that all need to have a function bound to an event.

This may not be the best place for it, but my initial attempt is to add the delegate (i.e. a click handler, using EventTarget.addEventListener() for the click event) in the componentDidMount method on the linksContainer element...

this.linksContainer.addEventListener('click', this.clickHandler.bind(this))

Then define that method:

clickHandler(clickEvent) {
    if (clickEvent.target.classList.contains('pageLink')) {
       //the lines in this block could be moved to a separate method
       let dir = (clickEvent.target.name === 'prev') ? -1 : 1,
           year = this.props.selectedYear + dir;
        this.props.onYearChangeHandler(year);
    }
}

The code inside that conditional if (clickEvent.target.classList.contains('pageLink')) { could be moved to a separate method. And that click handler could also detect clicks on other elements and delegate to other functions

See an example in this codepen.

\$\endgroup\$
4
  • \$\begingroup\$ Yep, addEventListener is the ticket \$\endgroup\$
    – konijn
    Commented Aug 24, 2017 at 17:53
  • \$\begingroup\$ interesting toughts, thanks :) +1 I'm too new to React, used ExtJS most of the time on my previous job, and there are almost no need of any HTML \$\endgroup\$ Commented Aug 25, 2017 at 6:35
  • 1
    \$\begingroup\$ This is really just a more roundabout way of doing the first option and probably less efficient as you need to wait for the event to propagate all the way up to the document before it gets handled. Furthermore, this could become a bit of a maintenance problem i.e. your at risk of introducing breaking changes because all your click handling would be in here. React promotes composition with the notion that components are self-contained (which should include event handlers etc.). I certainly wouldn't be hooking up events on the document unless I absolutely had to. \$\endgroup\$
    – James
    Commented Aug 25, 2017 at 8:46
  • \$\begingroup\$ okay - I added a ref to the links container (i.e. <span>) so as to avoid using the document object. What do you think of that? \$\endgroup\$ Commented Nov 8, 2017 at 1:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.