search

YahooArchive / fluxible-router

MOVED TO FLUXIBLE REPO
104 stars 26 forks source link

NavLink suggestion #39

Closed sAbakumoff closed 8 years ago

sAbakumoff commented 9 years ago

It's great that className and isActive were added in NavLink component properties: https://github.com/yahoo/fluxible-router/blob/master/lib/NavLink.js I suggest to add one more parameter which is tagName. Example : I would like that the whole row of Customers Table was link to the customer's page so I should be able to write something like

<NavLink className='app-table-row' tagName='tr' routeName='customer' navParams={customer.id} {...this.props} />
Vijar commented 9 years ago

I like the idea of allowing non anchor tags, but how would followLink behavior work in that case?

Perhaps we should provide a generic Nav component that can wrap arbitrary tags. @mridgway @lingyan thoughts?

gpbl commented 9 years ago

As much as I like the declarative syntax, imho this is a case where the onClick handler (eventually using the History API or makePath with the navigate action) fits better. NavLink should be only for links, e.g. a replacement for anchor tags. (however using the navigate action will scroll to the top of the page)

mridgway commented 8 years ago

This isn't something that we're considering adding. Closing.