Change Turbolinks unmount event from before-visit to before-render

[szyablitsky]

fixes #706

---

This change is 

[coveralls]

Coverage remained the same at 99.329% when pulling 666237a on szyablitsky:master into 75cf810 on shakacode:master.

[coveralls]

Coverage remained the same at 99.329% when pulling 9125420 on szyablitsky:master into 75cf810 on shakacode:master.

[coveralls]

Coverage remained the same at 99.329% when pulling 9125420 on szyablitsky:master into 75cf810 on shakacode:master.

[justin808]

@volkanunsal @szyablitsky Do we have consensus on merging this one?

[justin808]

---

Reviewed 2 of 2 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[volkanunsal]

Looks good to me. 👍

[justin808]

Imagine I have to explain this to a beginner or in an insturctional video. What do I say? That needs to go in the docs.

[szyablitsky]

@justin808 it is explained here https://github.com/shakacode/react_on_rails/pull/709/files#diff-66873aa717656c99137530d5cf817527R101

[coveralls]

Coverage remained the same at 99.329% when pulling 6feb067 on szyablitsky:master into 75cf810 on shakacode:master.

[coveralls]

Coverage remained the same at 99.329% when pulling 6feb067 on szyablitsky:master into 75cf810 on shakacode:master.

[coveralls]

Coverage remained the same at 99.329% when pulling 6feb067 on szyablitsky:master into 75cf810 on shakacode:master.

[coveralls]

Coverage remained the same at 99.329% when pulling 284c096 on szyablitsky:master into 75cf810 on shakacode:master.

[justin808]

A few comments...

My main concern is that this may not need to be configurable. I'm not convinced on why the default should be one way or another and why you'd change it.

We should provide a thorough explanation here:
https://github.com/shakacode/react_on_rails/blob/master/docs/additional-reading/turbolinks.md

---

Reviewed 4 of 6 files at r3, 2 of 2 files at r4.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

---

CHANGELOG.md, line 9 at r4 (raw file):

*Please add entries here for your pull requests.*
### Added
- Added an option to change Turbolinks unmount event from `before-visit` to `before-render`. To allow fixing "flickering" components when they are unmounted too early [#709](https://github.com/shakacode/react_on_rails/pull/709) by [szyablitsky](https://github.com/szyablitsky)

Please reference the github "issue" as well.

---

docs/additional-reading/turbolinks.md, line 102 at r4 (raw file):

## Unmount event
By default Turbolinks 5 will handle `turbolinks:before-visit` event for unmounting rendered components before visiting next page. If you use the same component on different pages this can cause the temporary disappearance of component during page load and even moving the parts of the page. To fix this issue switch unmount event to `turbolinks:before-render`.

@volkanunsal @szyablitsky What is the disadvantage to always using before-render? Why did we change this to before-visit?

---

docs/additional-reading/turbolinks.md, line 102 at r4 (raw file):

To fix this issue switch unmount event to turbolinks:before-render.
To fix this issue set the client side option turbolinksUnmountOnBeforeRender to true.

---

docs/api/javascript-api.md, line 37 at r4 (raw file):

   * Available Options:
   * `traceTurbolinks: true|false Gives you debugging messages on Turbolinks events
   * `turbolinksUnmountOnBeforeRender: true|false Binds unmount to before-render event

true binds unmounting to the before-render event, false keeps the default behavior of before-visit

---

node_package/src/ReactOnRails.js, line 62 at r4 (raw file):

   * Available Options:
   * `traceTurbolinks: true|false Gives you debugging messages on Turbolinks events
   * `turbolinksUnmountOnBeforeRender: true|false Binds unmount to before-render event

true binds unmounting to the before-render event, false keeps the default behavior of before-visit

---

Comments from Reviewable

[coveralls]

Coverage remained the same at 99.329% when pulling 052b662 on szyablitsky:master into 75cf810 on shakacode:master.

[coveralls]

Coverage remained the same at 99.329% when pulling 052b662 on szyablitsky:master into 75cf810 on shakacode:master.

[coveralls]

Coverage remained the same at 99.329% when pulling 052b662 on szyablitsky:master into 75cf810 on shakacode:master.

[szyablitsky]

Removed option and set unmount event back to turbolinks:before-render.

[justin808]

@volkanunsal Are you good with this change? I'd prefer not to have an extra option if it's not necessary.

[volkanunsal]

The reason I settled on before-visit initially is because it solved the error message introduced by before-render at the time I tested it for #610. We still don't fully understand why the double render error was being thrown by React.

[volkanunsal]

I'm ok with removing the configuration option.

[justin808]

---

Reviewed 5 of 6 files at r5.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

---

Comments from Reviewable

[justin808]

Fixed in 6.5.1.