Fix for issue #296 #297
Fix for issue #296 #297
Conversation
IE11 - nested views are not preserved in DOM when rerendered walmartlabs#296
Simply (naively?) stripping the line breaks from the output to be compared.
This reverts commit 7870d4f.
|
@solidgoldpig In addition to complimenting you on your amazing username, I'd like to thank you for this PR. @kpdecker do you have any profiling set up on any mweb projects where we could see the impact of this change? First thought is that if there is any impact we can do a conditional for IE, else use innerHTML |
|
I did think of putting in a conditional for IE11, but seeing that the only way appears to be matching navigator.agent for /Trident7./ I steered clear of it. Obviously, I understand if my personal dislike for that solution is trumped by considerations of real world performance. I also considered using this.$el.empty() but that would not only be slower, but would also require any events on the nested views to be redelegated when reinserted. Thanks for Thorax - love its opinionatedness ;) |
|
It seems like this should go into the ie.js source file rather than the normal impl. |
|
@solidgoldpig would you mind trying to move to our |
Uses horrible match on nav.userAgent to detect IE11
| @@ -1,5 +1,6 @@ | |||
| /*global _replaceHTML */ | |||
| var isIE = (/msie [\w.]+/).exec(navigator.userAgent.toLowerCase()); | |||
| var isIE11 = !!navigator.userAgent.match(/Trident\/7\./); | |||
There was a problem hiding this comment.
The check for IE11 is just a horrible regex against navigator.userAgent.
Perhaps rather than testing for the user string here, it would make more sense to do quasi-feature detection?
- create a view with a nested view
- update the view
- check to see if the nested view still exists
- update the methods if it doesn't
|
I've moved the fix to ie.js. I now realise that Thorax.View.on('before:append'... block no longer seems to be needed in IEs 9 and 10. On the other hand, without the overridden _replaceHTML, IEs 9 and 10 both fail the "view helper - ensure manually initialized child view is not destroyed if it goes out of scope in template" test. |
| } | ||
|
|
||
| // IEs 9, 10 and 11 will lose references to nested views if view.el.innerHTML = ''; | ||
| // Fixes issue #296 |
There was a problem hiding this comment.
@solidgoldpig can you make this a full URL to the ticket?
There was a problem hiding this comment.
@eastridge Done
|
@solidgoldpig looks good to me! Thanks so much. @kpdecker Does this look ok to you now? |
…hat treat it as a verboten keyword barfing
|
Released in 2.3.2 |
IE11 - nested views are not preserved in DOM when rerendered
#296