Remove "Globals" (AMD build prep) #1116
Conversation
…o the widget prototype.
…e build strips the main closure.
| return function() { | ||
| return this.each(function() { | ||
| if ( !this.id ) { | ||
| this.id = "ui-id-" + (++uuid); |
There was a problem hiding this comment.
Should this have spaces around ++uuid?
There was a problem hiding this comment.
I suppose based on yesterday's discussion it should. I didn't actually touch this line, but I can update it.
There was a problem hiding this comment.
Ah sorry, didn't catch that you hadn't touched it. It stood out because the rest of the file conforms.
|
👍 |
|
👍 as for the standpoint below. I have rebased my local UMD branch (related to #1029) on top of yours; built the bundle (using the new AMD builder); and found no lint issues, including no name collisions. |
| @@ -637,10 +637,12 @@ $.widget( "ui.slider", $.ui.mouse, { | |||
| newVal = this._valueMax(); | |||
| break; | |||
| case $.ui.keyCode.PAGE_UP: | |||
| newVal = this._trimAlignValue( curVal + ( (this._valueMax() - this._valueMin()) / numPages ) ); | |||
| newVal = this._trimAlignValue( | |||
| curVal + ( (this._valueMax() - this._valueMin()) / this.numPages ) ); | |||
There was a problem hiding this comment.
if the ship has sailed, ignore me, but in the case of this line break, I would imagine the final ) (and others like it) would be on its own line:
newVal = this._trimAlignValue(
curVal + ( (this._valueMax() - this._valueMin()) / this.numPages )
);
There was a problem hiding this comment.
I'm fine with that (I actually prefer it). I don't think we're going to get a tool that verifies it though. I'll make the change when I update the grouping parens in the other file as well.
There was a problem hiding this comment.
I don't need it to verify it, as long as it doesn't prohibit it.
|
Done with my review. Added two notes above, but nothing that really needs to change. |
|
Landed in master da185a6...9e6095a |
When we switch to AMD, our main closures for each file will be removed in the built file. This is a series of changes to avoid name collisions of variables when the closures are gone.
In some cases, the data was moved into the widget prototype to allow better customization by users. In other cases, the names were just prefixed with the component name to avoid collisions.
UI Core and Effects Core still have unprefixed variables/functions, but I'd like to split these files up when we switch to AMD. So I figured we can wait till the split before dealing with the variables.
Position had a lot of variables, so I just wrapped it in another closure for simplicity. We should come back at some point and clean it up. Why did we create
$.positionAND$.ui.position?Button wasn't touched. @arschmitz is in the process of writing the new version, and if we only have one file that doesn't conform, we're still safe from collisions.