Skip to content

Conversation

thomasboyt
Copy link

When stopPropagation is set, event.stopPropagation() will be called in the <Link />'s click handler.

I didn't add tests for this because I didn't see any for <Link/> yet; LMK if they're needed.

@ryanflorence
Copy link
Member

They are needed, desparately, but that is our problem 😢

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably default to true, yeah?

Also, @mjackson is a fan of the no-paren-just-indent-when-one-operation, if you're in there making changes.

@sophiebits
Copy link
Contributor

It seems like we could instead allow you to specify a click handler and only do the transition if you didn't call e.preventDefault() or return false.

@ryanflorence
Copy link
Member

Interesting.

A plain old link like this would require the same:

<div id="demo">
  <a href="#demo">demo</a>
</div>
demo.addEventListener('click', function() {
  alert('hooba!');
});

@thomasboyt what do you think? Be like ember or be like a regular anchor?

@thomasboyt
Copy link
Author

A generic click interrupt like @spicyj suggested, simulating a click handler on a traditional <a> tag, sounds pretty cool. Just so I have it right, sounds like you'd then be able to do:

/* This transition will never fire */
<Link onClick={ (e) => { e.preventDefault(); } }>Foo</Link>

/* nor will this one */
<Link onClick={ (e) => { return false; } }>Foo</Link>

/* This transition will fire, but the event won't bubble */
<Link onClick={ (e) => { e.stopPropagation(); } }>Foo</Link>

Seems like https://github.com/rackt/react-router/blob/master/modules/components/Link.js#L111-L118 would be changed to something like:

handleClick: function(event) {
  if (isModifiedEvent(event) || !isLeftClick(event))
    return;

  var allowTransition = true;

  if ( this.props.onClick ) {
    allowTransition = this.props.onClick(event);
  }

  // calling `preventDefault` in the onClick handler indicates that the 
  // transition shouldn't happen 
  if ( e.defaultPrevented === true ) {
    allowTransition = false;
  }

  event.preventDefault();

  if ( allowTransition ) {
    transitionTo(this.props.to, this.getParams(), this.props.query);
  }
}

Thoughts?

@ryanflorence
Copy link
Member

@thomasboyt yeah, exactly.

I like it, we're just trying to copy the behavior of a normal a tag, and they don't have bubbles="false".

@thomasboyt
Copy link
Author

Cool, reopening as #178

@thomasboyt thomasboyt closed this Aug 7, 2014
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants