-
Notifications
You must be signed in to change notification settings - Fork 511
Re-enables buttons and links after going back #385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hey @rafaelfranca and @lucasmazza, here is the PR and 3 hot dancers: 💃 💃 💃 It fixes the weird behaviour on Firefox, re-enabling links and submit buttons when going back on history, keeping the behaviour consistent across all browsers. Like we said on #357, Using the The suite is green on Firefox, Safari and Chrome, I do not have any other browsers but they are probably okay, would be great if someone else confirm that for sure. |
This looks good. Using an event handler is way more isolated than messing with attributes, timeouts or anything like that. I'll dig into the
@rafaelfranca @JangoSteve any thoughts? @sobrinho thanks a lot! ❤️ 💚 💙 💛 💜 |
Well, this issue doesn't happen on Safari, at least not on Safari 7. About intentionally disabled elements, it will be reenabled, but only for elements disabled by Although, if someone wants to disable this behaviour because of that, it's possible just removing the listener: $(window).off("pageshow.rails") If we really want to handle that, we may use another data to identify manually disabled elements and automatic disabled elements, but I think it's unnecessary. Other disabled elements won't be affected because |
// See https://github.com/rails/jquery-ujs/issues/357 | ||
// See https://developer.mozilla.org/en-US/docs/Using_Firefox_1.5_caching | ||
$(window).on("pageshow.rails", function () { | ||
$.rails.enableFormElement($($.rails.enableSelector)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of enabling all elements I think we should look through all elements matching the $.rails.enableSelector
and checking if they have been disabled by Rails just like we are doing with the $.rails.linkDisableSelector
ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sobrinho what do you think of this one?
FWIW I managed to reproduce this with Safari 7.0.6 (9537.78.2), OS X 10.9.4 and this HTML. |
Firefox has a weird behavior where when you hit the back button it keeps the submit buttons and links on disabled state which even reloading the page do not re-enables them. Using the pageshow event, which is triggered every time a page loads, we can keep the rails-ujs behavior consistent with all other browsers. See https://developer.mozilla.org/en-US/docs/Using_Firefox_1.5_caching [Fixes rails#357]
@lucasmazza take a look again :) |
@lucasmazza I'm still not able to reproduce on Safari using your gist. When I submit and hit the back button, the button is enabled as it should be. |
@sobrinho which Safari version? Same minor/patch numbers as mine? |
Exactly the same combo: Safari 7.0.6 (9537.78.2) + OS X 10.9.4 😐 |
@sobrinho accessing it through a local web server instead of the |
Re-enables buttons and links after going back
Reproduced on latest
|
Firefox has a weird behavior where when you hit the back button it keeps
the submit buttons and links on disabled state which even reloading the
page do not re-enables them.
Using the pageshow event, which is triggered every time a page loads, we
can keep the rails-ujs behavior consistent with all other browsers.
See https://developer.mozilla.org/en-US/docs/Using_Firefox_1.5_caching
[Fixes #357]