[11:00:18] hi all [11:00:21] <_|Nix|_> Hey! [11:01:18] hi [11:02:00] gseguin agcolom [11:02:08] hey :-) [11:04:01] I see some PR's on the agenda [11:04:16] hey [11:04:35] let's start with Panel widget [11:04:50] anything we need to discuss about those PR's? [11:04:54] <_|Nix|_> uGoMobi: Yeah, I'm mostly concerned about the swipe one. It has highlighted what could be a general problem with the way we attach event handlers. [11:05:20] <_|Nix|_> ... or rather the way we detach them. [11:05:27] sorry, could we have the link to the agenda again please [11:05:34] <_|Nix|_> https://docs.google.com/spreadsheet/ccc?key=0AskujzE4Ig0QdG5nSmZiSUhjYm4ya29CdjhLZmJwSWc&usp=sharing#gid=3 [11:05:41] Thanks :-) [11:06:04] <_|Nix|_> Basically, whenever we .on( "eventname", function() { } ); We can't simply .off( "eventname" ); [11:06:27] <_|Nix|_> We need to store the function in a variable and then pass it to off: .off( "eventname", theFunction ); [11:06:37] <_|Nix|_> Otherwise we risk removing other people's event handlers. [11:07:27] yeah, I see the problem [11:08:11] <_|Nix|_> I've changed the way the swipe-related mouse event handlers are attached, but the rest of touch.js also needs a going over, and so does the rest of the library. [11:08:51] _|Nix|_: i would just file an issue for it for now [11:09:00] _|Nix|_: we have never had an issue reported about this [11:09:02] <_|Nix|_> When I say "I've changed", I mean pending landing of https://github.com/jquery/jquery-mobile/issues/6946 [11:09:24] so while its worth attention i dont think its super high priority [11:09:32] <_|Nix|_> arschmitz: True, but it's still not good practice, if you ask me. [11:09:42] _|Nix|_: im not disagreeing [11:09:52] <_|Nix|_> arschmitz: Agreed, so we can do this half-passively, just like with this._on() [11:10:36] _|Nix|_: but you should file an issue so we dont forget [11:10:39] _|Nix|_: does need to be fixed before we can fix https://github.com/jquery/jquery-mobile/issues/6925 [11:10:39] <_|Nix|_> It shouldn't really be an issue inside widgets. [11:11:03] uGoMobi: this does not fix any issues its just not good practice [11:11:06] <_|Nix|_> uGoMobi: Yes. [11:11:26] <_|Nix|_> uGoMobi: Oh, I see what you mean. [11:11:40] <_|Nix|_> uGoMobi: The swipe event needs to be fixed for https://github.com/jquery/jquery-mobile/issues/6925 to be fixed. [11:11:51] right but not the issue with off [11:11:56] <_|Nix|_> uGoMobi: So, the fix for https://github.com/jquery/jquery-mobile/issues/6925 depends on the fix for https://github.com/jquery/jquery-mobile/issues/6262 [11:12:25] right [11:12:31] <_|Nix|_> ... and the fix for https://github.com/jquery/jquery-mobile/issues/6262 is implemented the right way - that is, swipe is not doing any .off( "eventname" ) anymore. [11:13:20] https://github.com/jquery/jquery-mobile/issues/6925 isn't a high priority issue if you ask me [11:13:35] so we could let this wait until 1.5 [11:13:44] so we have some time to look into all the events [11:13:47] what do you think? [11:14:05] uGoMobi: it was documented as just not possible in 1.3 [11:14:32] uGoMobi: we just said no sliders in panels if you use swipe to close [11:14:58] yeah I know [11:15:25] so this would be a new feture then [11:15:30] so it should wait for 1.5 [11:16:00] I agree [11:16:22] _|Nix|_: any objection? [11:16:44] <_|Nix|_> Naw. Let's do it for 1.5.0. [11:16:52] while we are talking about swipe i found a solution for the ios issue that does not effect our platform support [11:16:57] ok, let's set milestone to 1.5 on issue and PR's [11:17:03] <_|Nix|_> OK. [11:17:10] so we will get correct cords on all devices [11:17:34] <_|Nix|_> Well, actually, hold on ... [11:17:44] <_|Nix|_> https://github.com/jquery/jquery-mobile/issues/6262 might be considered a regression. [11:17:58] <_|Nix|_> ... because in 1.3.2 we let events bubble, so their default could be prevented. [11:18:11] _|Nix|_: we were only talking about 6925 [11:18:17] <_|Nix|_> Oh, OK. [11:18:19] <_|Nix|_> Sorry! [11:18:51] 6262 is a bug and a regression [11:19:34] arschmitz: awesome (re: iOS fix) [11:19:46] <_|Nix|_> arschmitz: If we choose to fix https://github.com/jquery/jquery-mobile/issues/6262 sooner than 1.5.0, then I think https://github.com/jquery/jquery-mobile/issues/6925 is fairly trivial, and it may be beneficial to have it. [11:20:04] _|Nix|_: has nothing to do with how beneficial it is [11:20:14] _|Nix|_: we documented that it was not support [11:20:22] so to do so now is a new feature [11:20:23] _|Nix|_: well, somehow it did work in 1.3 without the 6925 fix [11:21:03] <_|Nix|_> uGoMobi: It did work, because the swipe events were being triggered on the element that received the mousedown, not the element that had the handler attached. [11:21:31] then maybe that is what we need to revert? [11:22:11] <_|Nix|_> uGoMobi: Nono, because that's not right either. A sequence of mouse events should result in exactly one swipe event, not multiple. [11:22:18] the two issues are really unrelated [11:22:24] one is a bug and a regression [11:22:32] the other is a new feature [11:22:56] and just the feautre depends on the bug being fixed [11:23:01] <_|Nix|_> Wait a sec ... arschmitz is right ... https://github.com/jquery/jquery-mobile/issues/6925 should not have been possible in 1.3.2, because the panel wasn't looking for whether the default is prevented, so it would've closed on swipe. [11:23:06] yes, agreed. Discussion is about that _|Nix|_ says we would need the new feature to fix the bug/regression [11:23:29] <_|Nix|_> uGoMobi: Nono. https://github.com/jquery/jquery-mobile/issues/6925 -> 1.5.0, and https://github.com/jquery/jquery-mobile/issues/6262 -> 1.4.1 [11:23:31] uGoMobi: no he doesnt hes just saying its trivial after that [11:23:50] sorry, misunderstood [11:24:00] ok so i think we are all in agreement then? [11:24:12] I think so too [11:24:30] A note about our agenda document... [11:24:43] <_|Nix|_> arschmitz: People can .off( "swipeleft" ) on the elements to which the panel attaches the swipe handles and attach their own handlers. [11:24:50] let's only put things in there that we are going to discuss [11:24:58] so we can just go from top to bottom [11:25:34] I don't think we need a list of all PR's that are awaiting review [11:25:41] <_|Nix|_> OK. Removing ... [11:25:50] thanks [11:26:04] uGoMobi: agreed if any one is waiting on a review from me just ping me [11:26:08] ill do it asap [11:26:44] yeah [11:26:58] and as far as status of prs we can just check the prs them selves and see [11:27:22] Contacted coveralls to see if we can delay the posting of the coverage result [11:27:29] true [11:27:35] gseguin: cool [11:27:46] but if something needs discussion then of course add it [11:28:10] gseguin: anything about dependencies? [11:28:17] what about them? [11:28:32] it's now handled by bower [11:28:33] gseguin: I am just looking at the agenda [11:28:40] so if you want to change a version [11:28:45] update the bower.json [11:28:48] "one last PR from Ghislain to review + merge" [11:28:53] <_|Nix|_> arschmitz: Once https://github.com/jquery/jquery-mobile/issues/6946 lands, I can advise the OP of https://github.com/jquery/jquery-mobile/issues/6925 that they can remove the panel-attached swipe handlers and handle swipe manually, at least until 1.5.0. [11:29:01] run `grunt bowercopy` [11:29:15] git add, git commit, git push [11:30:10] is this documented somewhere? [11:30:35] uGoMobi: nope [11:30:43] where would you like it documented? [11:30:53] dependencies.md ? [11:31:04] scott_gonzalez: input? ^ [11:31:17] gseguin: other projects put that stuff in the readme i think [11:31:20] or readme.md [11:31:39] well it's more of a developer task than a user task [11:31:41] but sure [11:31:59] right [11:32:18] Core documents it in a comment inside grunt.js [11:32:25] Where the dependencies are listed. [11:32:34] gseguin: yeah there was a discussion about this with qunit a while back and it was about the readme i think [11:32:46] Ok, that's a lie. [11:32:49] scott_gonzalez: do you remember that? [11:32:51] Core *is going to* document it... [11:33:04] I don't recall any discussion about documenting this. [11:33:10] scott_gonzalez: so you're suggesting a big comment in the bowercopy configuration ? [11:33:14] hmm ok [11:33:16] Other than the recent discussion when core realized everything was borked. [11:33:23] <_|Nix|_> :) [11:33:52] * _|Nix|_ signs up for a course on agriculture [11:34:10] DaveMethvin: Where is the discussion that just happened about bowercopy and committing dependencies? [11:34:13] Was it just in IRC? [11:34:14] will this be the same for all projects? [11:34:25] uGoMobi: it should be [11:34:25] can't we centralize the documentation? [11:34:34] scott_gonzalez: on a commit, let me get it [11:34:47] uGoMobi: the problem is that nobody will read that doc until it's too late. [11:34:51] Which is what happened with core. [11:35:09] And I rolled my eyes and held and laughed as someone else said "I told you so" [11:35:12] https://github.com/jquery/jquery/commit/9ac88dea494b0b79d1e1d1d26f8d9f5eb670cf62 [11:35:13] this is what i was thinking of https://github.com/jquery/sizzle/issues/230 [11:35:18] :) [11:36:24] How about a comment in the grunt.js file that point you to the documentation? [11:36:36] and/or in the readme.md [11:37:10] Why don't we run bowercopy as part of the npm install script? [11:37:29] Because they're different things. [11:37:54] Bower dependencies are committed. [11:37:57] but in case of a release it would ensure that the right version is pushed to the repo at least at release time [11:38:06] scott_gonzalez: I understand that [11:38:10] bowercopy should only be run at the time of dependency changes. [11:38:15] it's really very similar to submodules, with many of the same drawbacks unfortunately [11:38:32] scott_gonzalez: I'm thinking of a failsafe way to have the updated dependencies pushed [11:38:35] TBH, it's not at all like submodules. [11:38:41] did we decide on bower partially because we hated sub modules lol? [11:38:51] well in that they get out of sync, not in the mechanics [11:39:03] Anyone who updates the dependencies and doesn't actually test that the update works is doing something Very Bad. [11:39:33] Anyone who does test and then commits without realizing they still have a dirty checkout is doing something Very Bad. [11:39:47] <_|Nix|_> Such updates will be submitted via PR as everything else, right? [11:40:07] _|Nix|_: Not for all projects. [11:40:21] _|Nix|_: for mobile yes [11:40:36] scott_gonzalez: I agree. And putting the bowercopy in the npm install script would be more of a safety net that will ensure that we never release with outdated dependencies [11:40:45] The problem with https://github.com/jquery/jquery/commit/9ac88dea494b0b79d1e1d1d26f8d9f5eb670cf62 is that gibson never tested. [11:41:14] gseguin: That's not a safety net. [11:41:19] That's a ticking time bomb. [11:41:30] Unless you have tests that are being run at the time that npm install is being run. [11:41:54] gseguin: yeah sorry but that sounds like a huge potential for trouble [11:43:07] alright [11:43:59] let's start with document it [11:43:59] <_|Nix|_> Umm ... how is this different from long ago, when we collectively decided to drop in another version of core, or another version of the widget factory? [11:44:15] documenting* [11:44:26] <_|Nix|_> We basically copied the file, ran the tests, committed and pushed. [11:44:31] <_|Nix|_> This sounds no different. [11:44:41] _|Nix|_: its a different process thats all [11:45:22] <_|Nix|_> arschmitz: Right, but it accomplishes the same thing. So, I think as long as we think about it the same way, we should be good. This shouldn't happen very often, and so we maybe don't need to automate it. [11:45:56] _|Nix|_: right thats what scott_gonzalez and i were saying [11:45:56] _|Nix|_: Yes, it's essentially the exact same thing, which is why I don't understand how there are issues. [11:46:23] The only difference is that you don't have to manually copy the new files anymore. [11:46:30] _|Nix|_: no difference except we have a file describing which versions we're using [11:47:03] <_|Nix|_> Right, so instead of copying a file, you edit bower.json ... sounds good to me. [11:47:43] <_|Nix|_> How about a big fluffly comment in bower.json urging one to tread carefully and test exhaustively. [11:47:49] <_|Nix|_> ? [11:47:58] it's json, not comments in json [11:48:05] <_|Nix|_> Gah! [11:48:10] ok so we just need to document the process of updating with bower [11:48:38] right [11:48:45] comment in the grunt file? [11:48:47] <_|Nix|_> gseguin: What about "xyzzy" : "BE CAREFUL! YOU MAY BREAK THE WORLD!" (Sorry about the caps) [11:49:11] <_|Nix|_> gseguin: Will unrecognized keys break the process? [11:49:11] whatever you find appropriate: readme.md, dependencies.md, wiki, Gruntfile.js [11:49:22] just let me know [11:49:34] _|Nix|_: I don't think so [11:49:43] gseguin: apparently core was saying not readme [11:50:09] <_|Nix|_> Ummm ... CONTRIBUTING.md ... ? *shrug* [11:50:16] in that discussion on the commit that dave pointed to [11:50:17] _|Nix|_: we can have a "comment" key in the bower.json [11:50:18] agcolom: right, so let's be consistent and put it at the same place as core [11:50:25] that sounds like a good place [11:51:05] see https://github.com/jquery/jquery/commit/9ac88dea494b0b79d1e1d1d26f8d9f5eb670cf62#commitcomment-5055737 [11:51:49] agcolom: yeah that's what I was saying, this is only for committers [11:52:00] so it doesn't really belong in the readme.md [11:52:24] I'm fine with putting a comment key in the bower.json [11:52:33] <_|Nix|_> +1 [11:52:47] -gseguin- "comment": [ "update process", [11:52:58] "1. blah blah", [11:53:04] and so on [11:53:04] <_|Nix|_> If dropping in new versions involves editing bower.json, then you should be confronted with the documentation during that unavoidable step. [11:53:06] ] [11:53:41] I am fine with that too but we should check with other projects because we should put at the same place [11:53:50] only few minutes left [11:54:01] yeah I've gotta drop off actually [11:54:01] so let's move that discussion to -dev [11:54:13] I'll see you on -dev [11:54:25] gseguin: do you have one more minute? [11:54:36] sure [11:54:49] <_|Nix|_> Let's dev. [11:54:55] * _|Nix|_ winks at scott_gonzalez [11:54:59] gseguin: just want to check last items on the agenda [11:55:25] unit tests, code climate, TravisCI [11:55:30] any updates here? [11:55:33] _|Nix|_: Pretty much anything can be a verb ;-) [11:55:52] TravisCI, last PR is about to land [11:56:04] unit tests, no progress [11:56:12] code climate, no progress [11:56:24] ok thanks [11:56:50] arschmitz: you had a solution for API docs examples without iframes, right [11:57:00] arschmitz: I mean, to prevent JQM taking over the whole page [11:57:26] uGoMobi: yeah just need to set $.mobile.gradeA=$.noop [11:57:37] short circuts everything [11:58:03] ok great [11:58:24] and I see we need to comment on the issue opened by agcolom [11:58:44] to provide a list of which widget can be used without page [11:59:23] "memory leaks and broke animation callbacks" [11:59:38] let's discuss that on -dev [11:59:44] <_|Nix|_> Yeah ... arschmitz rocked that one! [11:59:51] awesome [12:00:08] ok thanks all [12:00:12] see you on -dev [12:00:25] <_|Nix|_> L8R