Js refactor #745
Js refactor #745
Conversation
| @@ -0,0 +1,27 @@ | |||
| /* ======================================================================== | |||
| * Ratchet: modals.js v2.0.2 | |||
There was a problem hiding this comment.
yes thank you i have fixed it
|
I agree about using a different namespace as long as the rest of the guys agree too. PS. Don't forget to squash the patches. |
|
Oh, and do |
|
Ok I'll wait for any feedbacks on that to create a namespace for Ratchet. What do you mean by running |
|
OK, I guess I'll try to remember to run it manually, no big deal. |
|
I have squashed my commits, when this PR will be merged I'll work on fix push.js with |
|
How about the namespace change? Can you add it in a separate patch first? |
|
Yes sure I'll create another PR, to create a custom namespace for Ratchet |
|
I was thinking to have it in this PR and see how it'll look like. |
|
Oh ok sure, I was not sure if I'll add this on this PR or not, but ok I'll add the custom namespace on this PR. |
| @@ -502,6 +502,6 @@ | |||
| } | |||
| }); | |||
| window.addEventListener('popstate', popstate); | |||
| window.PUSH = PUSH; | |||
| window.rtc.push = PUSH; | |||
There was a problem hiding this comment.
This would break backward compatibility. I'd leave it available under the global and also add it to the namespace.
|
Otherwise looks good to me. |
|
Thank you for your feedbacks. And yes I'll add |
|
Not sure which one is better, Thanks @hnrch02 for the feedback :) |
|
As you want, but |
| @@ -503,5 +503,6 @@ | |||
| }); | |||
| window.addEventListener('popstate', popstate); | |||
| window.PUSH = PUSH; | |||
There was a problem hiding this comment.
Maybe add a comment here to remove this in the next major version?
There was a problem hiding this comment.
Yes it's better to keep that in mind.
Hello,
I created a common.js file to refactor some commons function.
We have talked about it with @XhmikosR in #491.
I added
getBrowserCapabilitiestowindowobject, but i think it would be better if Ratchet had a namespace for example:rtcAnd with that we will be able to do that:
or