search

turbolinks / turbolinks-classic

Classic version of Turbolinks. Now deprecated in favor of Turbolinks 5.
MIT License
3.54k stars 428 forks source link

Adds support for appending and prepending on partial page replacement #621

Closed alecdotninja closed 8 years ago

alecdotninja commented 8 years ago

Although the interface works slightly differently than requested, this would accommodate #563.

rails-bot commented 8 years ago

r? @Thibaut

(@rails-bot has picked a reviewer for you, use r? to override)

alecdotninja commented 8 years ago

Again, the issue with the specs on 209.17 seems to be upstream on Rails master.

Thibaut commented 8 years ago

Nice PR.

I'd prefer making prepend / append act like change in JS:

Turbolinks.visit(url, prepend: ['id'])
Turbolinks.replace(string, append: ['id'])

instead of having them be booleans.

alecdotninja commented 8 years ago

@Thibaut The API now matches what you have requested.

I should note, however, that exactly one of the append, prepend, or change options may be used in call to Turbolinks.visit or Turbolinks.replace.

Do you think there should be support for using several of these options at the same time?

alecdotninja commented 8 years ago

After some experimenting with my own projects, it quickly became clear to me that in order for the append and prepend options to be most useful, they should be allowed in combination with each other and the change option.

For example, when progressively enhancing to create an infinitely scrollable list, it is intuitive to want to append new entries to the list container while performing a change on a node at the bottom of the page containing pagination links. From this point, a small amount of unobtrusive JavaScript can be added to programmatically click the "next" link as it approaches the viewport.

The combination of the append and change options allows the easy creation of an infinitely scrollable list that seamlessly degrades when JavaScript is not available (and perhaps more importantly, remains indexable by search engines).

These changes are now reflected on my branch.

Thibaut commented 8 years ago

Sorry for the delay. I'll review this next week-end.

Thibaut commented 8 years ago

If you rebase the tests should pass.

alecdotninja commented 8 years ago

@Thibaut I have rebased to master and squashed my commits.

Thibaut commented 8 years ago

Thanks

naveed-ahmad commented 8 years ago

thanks @anarchocurious you did amazing job :)