[12:00:45] jzaefferer awayrxaviers fnagel [12:00:52] hey [12:01:28] hey [12:02:45] I guess the first thing on the agenda that we can discuss is datepicker. [12:03:04] ok [12:03:14] guess you've seen the PRs? [12:03:28] It's been a while (two months) since out last meeting, so we'll probably need to provide context for some stuff. [12:03:38] But on the last agenda, we had this item: [12:03:41] Need to add the date to the `select` and `change` events [12:04:20] Which is addressed by https://github.com/jquery/jquery-ui/pull/1786 [12:04:21] And is ready to land. [12:05:17] Yeah, I saw the PRs, but didn't get around to it until after jzaefferer had already reviewed. [12:05:33] I did a quick glance over since he had already approved and didn't see anything that stood out. [12:05:38] So I think they're all ready to be merged. [12:06:05] perfect [12:06:20] So let's just go over those other PRs to make sure everyone is caught up. [12:06:24] Remove `aria-hidden` from all cells: https://github.com/jquery/jquery-ui/pull/1788 [12:06:44] This was brought up by Jon Gunderson on the planning wiki. [12:07:03] Yeah, which is pretty strange as he requested adding this property some time ago. [12:08:39] In the worst case, we'll put it back again! Its a simple change... [12:09:02] that was my thought too [12:09:03] Presumably his first comment was based on what he thought was needed and this latest comment was based on actual testing. [12:10:20] `numberOfMonths` could not be changed after initialization: https://github.com/jquery/jquery-ui/pull/1787 [12:10:21] This one is also ready to merge. [12:10:54] fnagel: I assume you'll merge all of those soon. Do you know what you'll work on next? [12:11:09] Is there anything that needs discussion right now? [12:11:26] We have this question on the agenda: "Should we add an event for rendering? Example: changing month or year" [12:12:20] Do you recall whose question that was? I feel like it may have been mine. [12:12:29] I still think it could be useful, but I don't have any specific use cases in mind, so I'm not sure if it's actually needed. [12:12:35] not really, I would like to finalize a11y but still missing a concept how to handle the multiple month functionality and differences between datepicker and calendar widget. When that's implement we could finalize tests [12:13:08] Ok, so let's not add a render event until we have an explicit use case that isn't solved by an existing option or event. [12:13:42] scott_gonzalez: you wanted some event for each rendering which might just translate to calling refresh() [12:14:43] Well every navigation (e.g., clicking the previous month button), will do a completely new render of the dates, right? [12:14:55] most actions, yes [12:15:03] prev / next month for sure [12:15:25] So I was thinking that if someone was modifying the DOM to add additional functionality, they'd need a hook for when that code needs to run. [12:16:20] But without a specific use case in mind, it's impossible to say that the hook isn't already in place with one of the other events or options. [12:16:58] For example, `eachDay` would solve a set of needs. [12:17:10] `buttons` would solve another set of needs. [12:17:24] So a generic render event may not be beneficial. [12:18:15] I agree that eachDay should help for most use cases [12:19:45] Do you have a list of accessibility questions? Should we try to schedule a call with Jon Gunderson, Birkir Gunnarsson, Joseph Scheuhammer, Colin Clark, Hans Hillen, etc? [12:20:13] no specific questions yet, no [12:20:27] I'm sure we won't get all of them (and we don't need that many people), but if we reach out to all of our contacts, we can hopefully get two of them on a call. [12:20:34] nothing more that wiki todo item "a11y functionality" [12:21:00] Ok, so maybe we should start with just asking Jon to do another review after you've merged the PRs? [12:22:14] We should start with a review by Jon as I'm hardly in my office the next weeks due to a new client [12:22:25] ok [12:23:01] and: at this point I have no specific questions, it's more like we need some best practice I could work with [12:23:37] ok [12:24:20] So after you merge, why don't you respond to his comment telling him that `aria-hidden` has been fixed and asking for another review. [12:24:50] Looks like I've got a few bugs I was supposed to investigate before things got crazy at work. [12:24:53] thats the plan :-D [12:25:03] So I'll look into those after I finish reviewing PRs. [12:25:58] I will try to do some research and extend the wiki with some rough concept for handling multiple month so we have something to talk about with Jon, Hans, ... [12:26:20] sounds good [12:26:33] We have a PR to add jQuery as a dependency in `package.json`: https://github.com/jquery/jquery-ui/pull/1779 [12:27:07] Would love to have feedback from other team members on this one. [12:27:28] Kind of a long discussion in that PR, so I'll wait a few minutes for everyone to read through it. [12:35:05] Thoughts? [12:35:46] I understand why he wants to add jQuery and I'm aware of that npm "problem" but I have not enogh understanding and real world experience with much package manager to be helpful for this discussion. [12:36:21] ok [12:36:22] But help me a little: he wants to add jQuery to npm dependencies as jQuery UIs code relies on that. Seems valid to me. [12:36:22] jzaefferer? [12:36:23] I'd really like to hear from jzaefferer since he did the webpack example. [12:36:32] correct [12:37:33] But the real problem comes from the fact that `package.json` shouldn't tell you the dependencies for jQuery UI. [12:37:50] Because you really shouldn't be using `jquery-ui` as a module on its own. [12:38:11] and your objection is a) there a thousands of concepts, if we start to change for one, we won't be stop. b) it would require us (devs) to download jQuery as a dependency via npm (would not be used) AND bower (which we actually use) [12:38:13] right? [12:38:21] You should be using individual widgets. [12:38:22] And those each have different dependencies. [12:38:29] Right. [12:38:43] But I think the use of the entire repo as a single module is the real problem. [12:39:01] I *really* don't want users doing that. [12:39:16] "You should be using individual widgets." means using UI as dependency does not work currently, but UI.WIDGET does [12:39:22] Especially if they're using a dependency resolver. [12:39:44] Because if they have a build system that understands dependencies and creates packages, they should not be bundling jQuery UI as a single entity. [12:40:21] Right, and if you do that, the dependencies listed in `package.json` are meaningless. [12:40:28] Seems like a mix our of "npm does not allow FE dependencies" and "user like to use the whole repo, even when using a resolver" but technically the PR is valid [12:40:47] Mhhhh [12:41:45] I tend to say merge the PR as it does no real harm and is technically right (dependency graph wise). Revert when there is a general solution for solving FE dependencies. [12:42:13] And I fully agree with you when saying "I personally hate the situation that the community has created" [12:42:17] its a mess [12:42:21] But then we'll likely never revert. [12:42:22] And we'll just be encouraging people to do a Bad Thing. [12:42:37] a valid point [12:42:40] Honestly, I feel like this is a political battle. [12:42:58] but having a wrong dependency graph seems bad to me too [12:43:23] But the problem is that you only have a wrong dependency graph is you treat jQuery UI as a single entity. [12:43:32] And that's not something we want people to do. [12:43:42] which (again) seems a problem of the overall concept [12:43:46] This is a use case we want to strongly discourage. [12:43:47] agree [12:44:51] Yehuda actually had a good quote about this a few years ago when talking about Ember vs. Angular. [12:45:12] Something along the lines of doing the wrong thing should not be as easy as doing the right thing. [12:45:33] I wish I could remember which talk it was in. [12:46:20] regardless what we do, seems not like a satisfiying solution. Isn't there a recomendation by the nom guys for such cases? [12:46:35] seems like a problem we are not alone with [12:47:39] I think the npm "solution" is to generate a separate `package.json` for every widget, even if they're in the same repo. [12:48:25] Which would require us to do about 50 separate releases to npm. [12:48:33] For each version that we publish. [12:48:52] ok, thats not gona happen [12:49:40] right [12:49:44] It becomes insane to manage. [12:50:21] But that's basically what we need to solve everyone's problem today. [12:50:22] And I don't think it's really worth it when we have workable solutions already. [12:50:30] https://github.com/jzaefferer/webpack-jquery-ui [12:51:07] It just sucks that everybody uses different tools. And they all have slightly different requirements. [12:53:29] Well hopefully jzaefferer will chime in later. [12:53:33] Anyone have anything else to discuss? [13:00:10] sorry, I'm reading up now [13:00:31] Anyone from chassis here? [13:04:22] ?? [13:05:04] sumedhN: I don't think anyone from Chassis is here right now. [13:05:19] okay [13:05:26] scott_gonzalez fnagel: specifying jquery as a dependency in jQuery UI's package.json seems fine to me. Every widget depends on jQuery, so it makes sense to specify that dependency. When doing require('jquery-ui/widges/autocomplete') it'll fail if jquery isn't installed [13:06:38] And what version would we bundle? [13:06:51] PS: "Sam Widges" seems to be a sandwich place in London, otherwise "widges" doesn't seem to be a real word [13:07:19] bundle, as in what version we install in our repo via bower? [13:07:43] No, what version would we put in `package.json`? [13:10:34] And how do you deal with wanting to specify a specific version for your application? [13:11:18] Would we just specify >=1.7? [13:11:30] Minimum supported version seems good [13:12:00] If user wants a specific version, they put that as a dependency in their app, npm should dedupe it as long as they version range is compatible [13:12:38] But isn't this problematic for future jQuery releases? [13:12:43] I don't mind the extra node_modules/jquery for devs working on jQuery UI. Its only a single package with no dependencies of its own [13:12:53] Would we specify >= 1.7.0 < 4.0.0? [13:13:37] If we assume that they're going to break shit again, yeah, upper limit also makes sense [13:14:35] Ok, so here's a new question. [13:14:59] This problem only exists if you list `jquery-ui` as an explicit dependency, but don't list `jquery` as an explicit dependency for your app, right? [13:15:20] Or is that not the problem? [13:15:49] And the problem is just that the dependency resolve for the packager doesn't know that it needs to bundle the jQuery that you've already explicitly listed because it's not listed as a dependency of jQuery UI? [13:15:55] Actually, that seems like BS too. [13:16:21] What's the specific use case that causes this to be a problem? [13:17:50] I think its this one: "This problem only exists if you list `jquery-ui` as an explicit dependency, but don't list `jquery` as an explicit dependency for your app, right?" [13:18:21] So that's a completely invalid dependency graph FROM THE APP. [13:18:22] Isn't it? [13:18:23] You cannot use jQuery Ui without using jQuery directly. [13:18:30] hmm.... [13:18:51] Well, that might not be completely true, but I doubt people are actually using the classes directly. [13:18:58] Because I never see code like that. [13:19:21] https://github.com/jzaefferer/webpack-jquery-ui/blob/master/index.js#L10-L12 [13:19:40] My point being that you cannot have code like `$('#elem').datepicker()` without listing jQuery as a dependency so you can make the `$('#elem')` call. [13:20:19] You can import the widget and instantiate it directly, without import jquery [13:20:21] hmmm.... [13:20:43] Its not popular, but we do have that API [13:20:46] No issues with two different jQuery instnaces? [13:21:08] what do you mean? [13:21:24] If you end up with one jQuery version in jQuery UI and another version directly in your app. [13:21:34] The event systems won't play nice together. [13:21:46] But we're assuming that doesn't happen, right? [13:21:55] We're counting on npm to dedup properly. [13:22:42] yeah, npm3+ seems to do that properly [13:24:21] ok [13:24:50] Do you want to comment on the PR asking them to update the version and fix the spacing? [13:25:44] Or we can just switch to two space indentation in master and then just ask them to rebase and fix the version. [13:25:55] We did have agreement to switch to standard eventually, right? [13:26:55] I think we can just add the dependency in a separate commit, as the version range you had above, and close the PR with that [13:27:10] let me create a PR for that... [13:28:10] according to https://semver.npmjs.com/ ">= 1.7.0 < 4.0.0" is valid [13:31:28] https://github.com/jquery/jquery-ui/pull/1790 [13:35:53] Do you know if the spaces are ok? [13:35:56] https://github.com/jquery/jquery-ui/pull/1790/files#r97847004 [13:36:22] I feel like they should be, but the docs never show a space. [13:40:57] https://github.com/npm/node-semver/blob/master/range.bnf [13:41:02] Looks like space = OR [13:41:21] Oh, no, that's not right. [13:41:55] Looks like it doesn't support spaces [13:45:08] Well, I guess the JS implementation doesn't match the BNF definitions. [13:45:12] Because the spaces do work.