[00:00:41] (03CR) 10jenkins-bot: [V: 04-1] Create mixin for node that responds to node click events [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/103062 (owner: 10Robmoen) [00:09:21] (03PS3) 10Krinkle: Sample commit for Jenkins [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114661 [00:10:46] (03CR) 10jenkins-bot: [V: 04-1] Sample commit for Jenkins [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114661 (owner: 10Krinkle) [00:11:16] (03PS4) 10Krinkle: Sample commit for Jenkins [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114661 [00:12:17] (03CR) 10jenkins-bot: [V: 04-1] Sample commit for Jenkins [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114661 (owner: 10Krinkle) [00:14:07] (03PS1) 10Krinkle: jsduck: Have a default --meta-tags setting [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114666 [00:14:58] RoanKattouw: Can you merge ^? [00:15:34] Fails otherwise https://integration.wikimedia.org/ci/job/VisualEditor-jsduck/2/console [00:16:40] (03PS5) 10Robmoen: Create mixin for node that responds to node click events [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/103062 [00:16:53] (03Abandoned) 10Krinkle: Sample commit for Jenkins [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114661 (owner: 10Krinkle) [00:17:38] (03PS4) 10Robmoen: Add ClickableNode to MwImageNode [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/103063 [00:18:15] (03CR) 10jenkins-bot: [V: 04-1] Create mixin for node that responds to node click events [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/103062 (owner: 10Robmoen) [00:20:01] (03PS5) 10Robmoen: Add ClickableNode to ce nodes. [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/103063 [00:20:10] (03CR) 10jenkins-bot: [V: 04-1] Add ClickableNode to ce nodes. [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/103063 (owner: 10Robmoen) [00:20:33] (03PS6) 10Robmoen: Mixin ClickableNode with ce nodes that have tools [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/103063 [00:20:41] (03CR) 10jenkins-bot: [V: 04-1] Mixin ClickableNode with ce nodes that have tools [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/103063 (owner: 10Robmoen) [00:21:59] (03PS7) 10Robmoen: Mixin ClickableNode with ce nodes that have tools [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/103063 [00:22:24] Arg, that should all be rebased [00:22:32] (03CR) 10jenkins-bot: [V: 04-1] Mixin ClickableNode with ce nodes that have tools [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/103063 (owner: 10Robmoen) [00:25:20] Clearly not [00:25:21] rmoen: could you merge https://gerrit.wikimedia.org/r/114666 ? [00:25:40] it's the only commit that'll pass jenkins bot. [00:25:46] Krinkle: gladly [00:26:13] (03CR) 10Robmoen: [C: 032 V: 032] jsduck: Have a default --meta-tags setting [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114666 (owner: 10Krinkle) [00:26:16] thx :) [00:27:01] rmoen: you'll also see that from now on it no longer initiates a ruby-lint job for patches that don't touch any rb files [00:27:09] (03Merged) 10jenkins-bot: jsduck: Have a default --meta-tags setting [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114666 (owner: 10Krinkle) [00:27:09] should speed it up a bit [00:27:54] Krinkle: Speeding up is good. [00:28:03] (03PS6) 10Robmoen: Create mixin for node that responds to node click events [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/103062 [00:28:10] (no-op rebase to trigger jenkins) [00:40:01] Krinkle: Is there a public set o' docs for JSDuck style that's not in the VE repo? [00:40:10] Like...if I wanted to use it for not-VE and have it carry any weight [00:43:27] Krinkle: Also, it seems silly not to have one big unified place for documentation when VE and MMV both use loads of mw classes...we could just generate them all together instead [00:43:37] Or figure a way to cross-compile them somehow [00:44:22] rdwrer: Except for the last part (combining extensions), I did not understand your question. [13:02:44] (03PS1) 10Alex Monk: Don't show edit summary preview if it's blank [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114737 [13:16:53] (03CR) 10Esanders: [C: 04-1] Don't show edit summary preview if it's blank (031 comment) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114737 (owner: 10Alex Monk) [13:17:51] (03CR) 10Esanders: [C: 032] doc: Use lowercase types where primitives (not objects) are expected [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114628 (owner: 10Krinkle) [13:18:09] (03PS2) 10Krinkle: doc: Use lowercase types where primitives (not objects) are expected [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114628 [13:19:51] (03CR) 10jenkins-bot: [V: 04-1] doc: Use lowercase types where primitives (not objects) are expected [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114628 (owner: 10Krinkle) [13:24:37] (03CR) 10Esanders: [C: 04-1] doc: Use lowercase types where primitives (not objects) are expected (031 comment) [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114628 (owner: 10Krinkle) [13:29:36] (03CR) 10Esanders: Create mixin for node that responds to node click events (033 comments) [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/103062 (owner: 10Robmoen) [13:31:31] (03CR) 10Esanders: [C: 032] Update OOjs UI to v0.1.0-pre (93f94e059f) [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114525 (owner: 10Trevor Parscal) [13:32:32] (03Merged) 10jenkins-bot: Update OOjs UI to v0.1.0-pre (93f94e059f) [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114525 (owner: 10Trevor Parscal) [13:32:59] (03CR) 10Esanders: [C: 032] Add autosize option to OO.ui.TextInputWidget [oojs/ui] - 10https://gerrit.wikimedia.org/r/114522 (owner: 10Trevor Parscal) [13:33:28] (03Merged) 10jenkins-bot: Add autosize option to OO.ui.TextInputWidget [oojs/ui] - 10https://gerrit.wikimedia.org/r/114522 (owner: 10Trevor Parscal) [13:35:35] (03PS1) 10Esanders: Update VE core submodule to master (31fb320) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114742 [13:38:23] (03PS4) 10Esanders: Make template parameter value inputs autosize [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114527 (owner: 10Trevor Parscal) [13:38:59] (03CR) 10Esanders: [C: 032] Make template parameter value inputs autosize [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114527 (owner: 10Trevor Parscal) [14:17:42] (03PS2) 10Alex Monk: Don't show edit summary preview if it's blank [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114737 [14:49:03] (03CR) 10Esanders: [C: 04-1] Don't show edit summary preview if it's blank (031 comment) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114737 (owner: 10Alex Monk) [17:04:41] heya James_F [17:05:53] Hey mooeypoo. [17:07:23] James_F, I added a 'Set to default thumbnail size" button but it will probably need a change of text. [17:07:30] https://gerrit.wikimedia.org/r/#/c/114420/3/modules/ve-mw/i18n/en.json [17:08:00] mooeypoo: We have "Make full size" already, right? Maybe "Make default size" for now, and maybe change it later? [17:08:46] ok, I wasn't sure if we want to emphasize this is the default *thumbnail* soze [17:08:47] size [17:09:06] which leads me to another question: Should it be set to that when a user changes an image from anything to 'thumb' type? [17:13:35] RoanKattouw, can you point me towards where the DM gets the initial image size, in case an image is set without one in wikitext? I have a discrepency between already-existing images and newly-inserted images' sizes. Is this information we get from Parsoid? [17:13:54] I can't find why VE loads those images slightly bigger than the php parser (and hece, bigger than the actual default size) [17:16:20] What does the Parsoid HTML look like? Does it have width and height attributes set? [17:16:56] I just checked again [17:17:05] TrevorParscal: Good morning; there's a discussion about HTML templating (Gabriel and CScott came up with something that looks semi-reasonable) over in #wikimedia-office [17:17:28] Parsoid's initial numbers don't make sense :\ but since the whole 'default size' is a bit confusing, can I verify with you? My wiki's default is set to 180px -- that's supposed to be the bounding box, correct? [17:18:10] so, on wikitext [[File:someimage.jpg|thumb]] parsoid returns an image with dimensions 165x220 <-- that shouldn't happen. It should be something x 180 [17:18:47] Yeah, so Parsoid disobeys the thumb size setting [17:18:52] Both wiki default and user settings [17:18:55] isn't that right? I've been searching the entire docs to see if the 180px is bounding box or just height, I can't seem to find a definitive statement, it's as if it's "obvious" that it's one way or the other. But since the image appears bigger than php parser, I assume I am right [17:19:01] * mooeypoo nods [17:19:05] (Because the wiki default is implemented as the default for the user setting) [17:19:15] 180px is a bounding box is my understanding [17:19:26] yeah it seems so. So it bounds whatever dimension is the bigger one [17:19:38] The default value for the thumb size is 220, Parsoid is using that as a bounding box apparently [17:19:51] good morning [17:19:57] It *looks* like that's what it's doing throughout, and that's how I made VE behave, but it isn't actually stated firmly anywhere in the docs. [17:20:14] Hey TrevorParscal. [17:20:39] RoanKattouw, in my local wiki, the php parser shows the image as smaller (width x 180) as it should. So it seems to be a Parsoid miscalculation. [17:20:58] Yes [17:20:59] also, tat results in the fact that when we save a page with images like that, we get wikitext of |NaNxNaN]] [17:21:03] Parsoid simply disrespects your settings [17:21:04] mooeypoo: Hmm. If you change image from inline to a block size, it probably should use the default size, yes. [17:21:05] ha! [17:21:07] so, I think those might be two bugs [17:21:07] Whoops [17:21:15] yeah, because VE doesn't recognize these images as default [17:21:17] Yeah almost certainly two different bugs [17:21:19] because they don't fit the size [17:21:53] so *technically* NaNxNaN shouldn't happen when images are default, if we tag them as attributes.defaultSize = true -- but VE doesn't catch those, because the size is not right. [17:22:25] James_F, what about block to block, like framed vs thumb, etc? should I touch the size? [17:22:27] Why does VE care what size they are? [17:22:53] mooeypoo: No, default only works for block-level, I think. [17:23:14] mooeypoo: So it should be greyed out for those cases; when it gets enabled for the first time, it should be set. [17:23:23] hm, okay [17:23:41] mooeypoo: But if in-edit you make it block (custom size) -> inline (custom size) -> block, it can retain the size if that's easier. [17:24:07] James_F, yeah when inline->block and back is active, we can do that pretty easily [17:24:21] Cool. [17:24:36] But yes inline -> block should in general set the size to default. [17:24:38] mooeypoo: If the defaultSize bit depends on the size being precisely right, that's not very robust, I feel like there must be a better way of doing that [17:24:45] Unless I'm missing something [17:24:47] Agreed. [17:25:14] James_F, ok, so if sizes are completely unset, there are tons of problems with images and scalable and such. So images should have a set size. But that meant that there was very limited ways of figuring out if the image is default or not [17:25:57] that's why I went with the bounding box. If it is in the bounding box, it's default size. User either clicked the 'default size' button or inserted the default size anyways [17:26:16] But what if it's actually been manually set to that size? [17:26:37] We don't want to dirty-diff it to default size (in particular, because it may have been done intentionally). [17:26:59] Yes, for the moment, it will consider it default, which isn't perfect. Problem is, most of the issues are due to the separation between all the size elements in DM/CE and then image vs scalable vs the size widget [17:27:25] * James_F nods. [17:27:28] The only other solution I can think of is to follow the same idea of the position [17:27:41] have a little checkbox saying "Set to specific size" and if it's unchecked, it's defaulted [17:27:57] so that would be for the size widget [17:29:50] mooeypoo: Does Parsoid not have a class for this? [17:30:04] And why do we need the defaultSize flag in the DM? Do we need to send Parsoid some sort of class to indicate default-ness? [17:30:23] it does [17:30:54] mooeypoo: We could do that. [17:31:12] aye, but then again we're in the issue of when to set and unset it ourselves in VE? [17:31:26] the button sets it, and a change in dimensions automatically unsets it? [17:31:38] Do we go by checkbox? [17:32:09] The image is inserted with that flag. That works fine. The problem is how to recognize if after a certain change the image is *still* "default" rather than specific size. [17:32:20] Right [17:32:22] Yeah. [17:32:24] I think: [17:32:25] So your problem is this: [17:32:33] * James_F shuts up and lets RoanKattouw talk. [17:32:34] 1) Image comes in with dimensions 220x185px and default class [17:32:43] 2) User changes image to 440x370px [17:32:49] 3) User changes it back to 220x185 [17:33:01] 4) DM needs to serialize this back to HTML and needs to somehow know to add the default class [17:33:08] (Whereas if #3 is skipped, DM needs to not add it) [17:33:13] No no no. [17:33:20] mooeypoo: Is --^^ a correct statement of the problem? [17:33:21] (add to that that atm, 220x185 is *not* default size, that's a parsoid issue) [17:33:25] Sure sure [17:33:27] Yes. [17:33:28] But ignoring that for now [17:33:31] 2) User changes image to 440x370px {and so implicitly unsets default size} [17:33:39] Yes but another issue [17:33:46] 3) User changes it back to 220x185 {by sayings "set to default size"} [17:34:03] 3) User changes it back to 220x185 --> does the user MEANS for it to be default or just this specific size? [17:34:09] yes exactly [17:34:26] Steps could instead be 3b) User changes it manually back to 220x185 {and so does not re-set as default size} [17:35:00] OK, so you're telling me there's a user intention we have for default size? [17:35:01] So a size-change event on an image /always/ unsets default flag on the DM, /except/ if the event is "set-default-size". [17:35:12] Doesn't that solve the entire problem? [17:35:15] Yes. [17:35:38] James_F, should we have it as a button, then, or as a checkbox that disables the size widget? [17:35:45] following the same idea of position [17:36:08] mooeypoo: Following the same as position seems consistent, but we already have a button, so it's inconsistent. [17:36:11] * James_F isn't sure. [17:36:21] mooeypoo: What do you think makes most sense for a user? [17:36:39] mooeypoo: (Maybe we could change the checkbox for position to a button that gates the other controls?) [17:37:16] Yeah, I've been thinking about that for a while. I think a button is good *IF* it's just to reference to the user "this is default", but that's NOT what we want to do. We want to basically flag that image as default size for the specific wiki (take the size element out, really) -- so in that case, I think the checkbox makes more sense. [17:38:08] to be perfectly honest, I think the most clearest thing would be an option button.. "Default Size" / "Set Size: [widget]" [17:38:30] But UX/UI designers could probably answer this best. [17:39:15] mooeypoo: We already have one "make this a size where I don't enter numbers" button, though. [17:39:28] (03PS3) 10Krinkle: doc: Use lowercase types where primitives (not objects) are expected [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114628 [17:39:31] No, the buttons are "fill this out for me" [17:39:38] I can change after it's filled out, it's more of a reference [17:40:01] It's not "Set this to max size" and now I can't change it again. Does this make sense? That's how it felt to me when I was using it. [17:40:08] mooeypoo: So a control cluster like [Make default size] | Set size: Width [ ] x Height [ ] [Make full size] maybe? [17:40:15] * James_F nods. [17:40:19] Yes, that's what I think is best. [17:40:33] (03CR) 10Esanders: [C: 032] doc: Use lowercase types where primitives (not objects) are expected [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114628 (owner: 10Krinkle) [17:40:43] Do we want to ask a UX person? [17:41:18] mooeypoo: Would we change the checkbox to a gating button in position too, for consitency [17:41:21] ? [17:41:23] i feel I'm needed here - sorry, talking in the other channel about crap [17:41:34] TrevorParscal: Yeah, your input welcome when your meeting is over. [17:41:38] ok [17:42:05] i think i'm done [17:42:18] they seem to be making a lot of mistakes, but there's no saving them [17:44:55] ok, I think I understand the issue here [17:46:07] VE UX Guideline #1: "The user should be aware of making, and explicitly chose to make, all changes in the resulting diff" [17:46:31] IT could be a trigger button, like the one we have with position, just when "Choose size" is checked, the size widget is enabled, or.. something. [17:46:47] * mooeypoo nods Fair enough. [17:46:49] yes [17:47:16] so the idea of a secondary "set size" tick, or something similar, is important [17:47:22] as you change the size it automatically ticks it [17:47:36] That makes sense. [17:47:42] if you untick it the size is reset to default size and we remove size info from parsoid [17:47:46] TrevorParscal: (Out of curiosity for later, when you're done with this topic: what are they doing that you'd characterize as a mistake?) [17:47:53] TrevorParscal: Hmmmmmm. [17:48:03] I'll have to add that to resizable nodes too, if a user changes size in the handles. [17:48:09] TrevorParscal: The default (ho ho) action we want to encourage is default-sized images, though. [17:48:13] the round trip case you mentioned is one, the other is taking a specifically sized image and making it default, either should be possible and the user should know what they are doing [17:48:33] in the case of resizable nodes, we don't have to complicate it with this checkbox [17:48:36] don't add it there [17:48:55] TrevorParscal, no no, not a checkbox, but a removal of "default" in case they're changed [17:49:06] TrevorParscal, I was trying to avoid touching all of that, which is why I did the test in DM [17:49:14] *BUT* that "guesses" the user intention which is a nono. [17:49:15] we can either choose to auto-auto it, or have a rule that resize handles create overrides even if it's the same size (I'm in favor of the latter) [17:50:02] yeah, just something to take off the defaultSize=true flag. CE can then deal with adding and removing the class [17:50:08] yeah, so, I think the resize handles should be exactly equiv to editing the size in the media size widget numeric inputs [17:50:24] yeah [17:51:18] TrevorParscal, now, question: Do you think the size widget should be enabled always to let the user change values, but if the user changes them, the option switches from "Default size" to "Set custom size" or something ---- *OR* do you think that if the "Default size" is checked, the widget is disabled, and the user must click the 'custom size' to enable it? [17:51:23] TrevorParscal: I worry that you're focussed on the fact that users /can/ make images different sizes, rather than the need to discourage them unless they really want to avoid default sized, though. [17:52:05] yeah, that relates to James_F point. Maybe if the widget is disabled, it shows the user they *actively* need to set "custom" to change size. Does this make sense? [17:53:04] I'll also report the two Parsoid bugs, but they are unrelated. My failed attempt just exposed them. [17:53:15] mooeypoo: when the set size is ticked, the numeric values are actual values in the numeric inputs, when it's not ticked the default numeric values are placeholder text in the numeric inputs [17:53:46] that is exactly what placeholders convey, and we use that other places (category title keys) [17:53:57] TrevorParscal, but the user can interact with the widget either way, or do we disable the widget, making the user explicitly choose "custom" before changing default size? [17:54:12] no need to disable it [17:54:19] The question is more towards what we want to be "encouraging" users to do, I think [17:54:42] the inputs will be blank, but have placeholder text, which perfectly conveys "if you don't input something, this is what will be used" [17:55:07] oy, blank inputs in size widget is trixy. It'll get all red. [17:55:21] I might have to add 'default size' to the widget [17:55:23] sometimes people use placeholders and labels, but that really only works well with search field, placeholders are meant to be default values [17:55:35] * mooeypoo nods [17:55:44] it shouldn't be red if the set size is not ticked [17:56:01] TrevorParscal, part of the issue here is that since I *know* you guys are working on a refactoring of all of these things, I don't want to change too much of the underlying size/scalable/image stuff [17:56:22] and they SHOULD be red if it is and they are blank, but you should fill them with the default numbers at them moment when the set size gets ticked [17:56:42] TrevorParscal, yes, it should, but it won't right now. I'll have to adjust the widget for it to happen. I can totally do that, but as I said, I was trying to avoid making too many code changes to these underlying things since I know your refactor will be changing them completely anyways. [17:57:11] well, don't let me hold you up - all I was doing was starting work on the standalone image model and trying to split the media edit dialog into pages [17:57:15] those things need to be completed [17:57:27] but I think this work should not conflict [17:57:31] * mooeypoo nods [17:57:46] ok. There's a bit of the sections that could use some refactoring so they work better with one another [17:57:48] what happens in the widget stays in the widget :) [17:58:31] (03CR) 10Catrope: "In the future PLEASE use the standard commit message format that we've been using for every single cross-repo sync for a while now. There'" [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114525 (owner: 10Trevor Parscal) [17:59:10] (03CR) 10Catrope: [C: 032] Update VE core submodule to master (31fb320) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114742 (owner: 10Esanders) [17:59:36] yeah, so let's recap the behavior: there is a control to explicitly enabled/disable sizing, adjusting the size in any way enables sizing, blanking the field disables sizing, disabling sizing blanks the fields, enabling sizing populates the fields with default values, when disabled the fields have placeholder text of deauflt values [18:00:07] That will also solve the 0x0 bug --> 0x0 could just be "Default". [18:00:34] (03Merged) 10jenkins-bot: Update VE core submodule to master (31fb320) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114742 (owner: 10Esanders) [18:00:53] does Resiable currently support undefined values for the current dimensions? [18:01:01] as in "unspecified"? [18:01:04] it should [18:01:08] the widget doesn't [18:01:12] it calls it invalid [18:01:25] Not sure about scalable [18:01:28] ok [18:01:37] (03Merged) 10jenkins-bot: Make template parameter value inputs autosize [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114527 (owner: 10Trevor Parscal) [18:01:40] that is something to consider, edsanders?? ^^ [18:01:44] * TrevorParscal looks [18:02:55] (03CR) 10Catrope: [C: 031] "@Krinkle: Any reason you didn't +2 this?" [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113544 (owner: 10Esanders) [18:02:56] well, it seems to assume you are passing an object with width and height keys, which is fine [18:03:38] the ratio seems to be undefined safe [18:03:45] * edsanders catches up [18:03:56] TrevorParscal, are you talking about scalable or the size widget? [18:04:02] Scalable [18:04:36] edsanders: all you need to know is that I'm trying to get Scalable to understand the difference between current dimensions being specified or intentionally undefined [18:05:28] you want to go from specified back to undefined? [18:06:17] I guess that's okay, you may need to consider the effect on a computer aspect ratio [18:11:04] (03CR) 10Krinkle: "I didn't test it. Patch makes sense, but haven't dealt with mediawiki.page.startup since I introduced mw.hook so didn't want to merge with" [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113544 (owner: 10Esanders) [18:14:24] (03PS3) 10Alex Monk: Don't show edit summary preview if it's blank [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114737 [18:15:19] (03CR) 10Alex Monk: Don't show edit summary preview if it's blank (031 comment) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114737 (owner: 10Alex Monk) [18:19:06] James_F|Away: hm.. seems the worklist doc is currently visible to anyone with the link. should we restrict it? [18:19:28] edsanders: yes, in certain cases, so the aspect thing needs to use either the current or default dimensions, whichever is set, preferring the former [18:20:03] Krinkle: I don't think it's top-secret, but it's not something we should have advertise and put on wiki [18:21:41] (03CR) 10Catrope: Don't show edit summary preview if it's blank (031 comment) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114737 (owner: 10Alex Monk) [18:21:57] Krinkle: Can https://gerrit.wikimedia.org/r/#/c/113544/ be merged? [18:22:13] I just took the liberty to change visibility for non-invitees from "Anyone with link" to "Anyone logged-in from Wikimedia Foundation", invites still rule so @gmail/wikia etc. people can still read/edit. [18:22:20] RoanKattouw: I replied [18:23:42] I see, sorry, missed that [18:24:17] Krinkle: I haven't tested it either, so do you mind taking this one? Then I can help Rob with his TOC stuff [18:25:09] OK [18:25:30] Hm.. is there supposed to be an accesskey on the toolbar/"Save page" and/or mwsave/"Save page" button? [18:25:38] I don't see the attribute nor the tooltip [18:25:52] Yeah those are supposed to have accesskey=s [18:25:55] Try Alt+S [18:25:58] Or whatever it is on mac [18:26:29] I'm assuming we don't override the trigger, right? So whatever the browser default is for accesskey [18:26:38] (ctrl-option on Chrome/Mac) [18:26:47] (unlike what we usually do for buttons) [18:27:04] I don't see that on master nor after that patch. [18:27:16] Doens't seem a race condition either, the attribute isn't there. [18:27:16] Hmm [18:27:25] Yeah I grepped for accesskey and it's not there [18:27:29] Did that code not get merged? [18:27:53] Guess not? [18:27:58] It's not in mw.messages [18:27:59] No https://gerrit.wikimedia.org/r/#/c/110434/ was merged [18:28:03] (accesskey-save) [18:28:11] That commit just isn't based on it [18:28:35] Hm.. [18:28:39] (03PS4) 10Esanders: Ensure VE init runs after mw.util.init [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113544 [18:28:44] (rebase) [18:28:46] Master has accesskey="s" on the toolbar button [18:28:51] Took a harder refresh [18:28:52] Try that one [18:29:03] and in the dialog as well [18:29:16] It doens't hit that race condition for me, but I'll just make sure it still works in that case. [18:29:26] We should probably do that tooltip thing as well. [18:29:58] Right now our buttons have no tooltips [18:30:05] anyway, goodtogo [18:30:14] (03CR) 10Krinkle: [C: 032] Ensure VE init runs after mw.util.init [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113544 (owner: 10Esanders) [18:31:26] (03Merged) 10jenkins-bot: Ensure VE init runs after mw.util.init [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113544 (owner: 10Esanders) [18:32:15] RoanKattouw: Can you re+2 https://gerrit.wikimedia.org/r/#/c/114628/ ? [18:32:20] Seems it got lost in zuul somehow [18:32:53] Hi guys, I got a quick question [18:33:11] would it be possible to run visual editor on v1.17 of mediawiki ? [18:35:00] morg: Not likely; we've been adding things to MediaWiki to let VisualEditor work (code for VisualEditor support has been added in parts in versions 1.20–1.22). [18:35:06] morg: Sorry. [18:36:12] No worries, thank you. [18:40:37] RoanKattouw: TrevorParscal: https://gerrit.wikimedia.org/r/#/c/112132/ [18:40:43] (03CR) 10Catrope: "This uses FieldLayouts without a FieldsetLayout, is that intended?" [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114535 (owner: 10Esanders) [18:41:10] Krinkle: Yeah I know [18:41:16] It seemed reasonable to me [18:41:27] Krinkle: But we should figure out if we can make this a bit less of a breaking change [18:41:46] Either that or convince ourselves that no one except VE is using Window subclasses [18:42:40] Well, it's a breaking change that should be a breaking change. Radically removing an integration that is currently nested in the constructor. [18:43:07] Right yeah [18:43:07] There is a way around it, but that involves making the future ugly instead of the past (e.g. wrapper classes, or null skip arguments) [18:43:20] So we should see if anyone else uses Dialogs or Windows [18:43:23] Probably not [18:43:33] And then merge this, merge it in core, merge a fix in VE [18:43:39] besides, I don't think anyone is using oojs-ui elsewhere yet where we can't help out to migrate things in time. If not, someone did something irresponsible. [18:43:46] (03PS5) 10Catrope: [BREAKING CHANGE] Refactor dialog and window sets [oojs/ui] - 10https://gerrit.wikimedia.org/r/112132 (owner: 10Krinkle) [18:44:00] RoanKattouw: Thanks. [18:44:11] rdwrer: ^^^ [18:44:16] Wat [18:44:18] Oh. [18:44:24] * rdwrer will...deal with that later [18:44:30] rdwrer: I /think/ you're not using dialogs yet, right? [18:44:33] rdwrer: Only applies if you're using dialogs [18:44:40] Nope [18:44:41] Or rolled your own subclasses of OO.ui.Window [18:44:44] We won't be, I think [18:45:00] OK, then you're fine. [18:45:05] In that case, good to merge. [18:45:07] https://github.com/search?q=OO.ui.Dialog+%40wikimedia&type=Code [18:45:11] https://github.com/search?q=OO.ui.Dialog+%40wikimedia&type=Code [18:45:15] https://github.com/search?q=WindowSet+%40wikimedia&type=Code [18:45:20] global search ftw [18:45:40] True. [18:45:48] though we use it a lot :P [18:46:04] and there's duplicate repos because, gerrit. [18:46:11] or rather, our config. [18:46:55] why don't we undupe them in our gerrit puppet config? I think this is being intentionally done right now, because someone said one shoudl be able to map the repo name and get one on github, even if we want a different one there. [18:47:05] Is that really worth it.. [18:48:16] Krinkle: Priorities. [18:48:46] Stuff adds up making a lot of things just a bit too hard to do, one of these days.. [18:49:08] can't we have another 1000 engineers, or ask if we can use Google's AI to do our work ? [18:49:13] Krinkle: This is why it was someone's full-time job. Argue with Platform why it's not any more. :-) [19:00:57] TrevorParscal: Have you reviewed https://gerrit.wikimedia.org/r/#/c/112132/ ? [19:01:11] lookin [19:01:12] (03CR) 10Catrope: "This needs to be rolled out carefully because it's a breaking change." [oojs/ui] - 10https://gerrit.wikimedia.org/r/112132 (owner: 10Krinkle) [19:01:24] yeah, so I helped write it [19:01:30] Oh OK [19:01:38] I looked through it earlier and it looked reasonable [19:01:40] Timo and I did it together on a single computer [19:01:45] Haven't done as detailed a review as I should, and I haven't tested it yet [19:01:46] yes, it has my support [19:01:47] Aha :) [19:01:51] Then I'll review it later today [19:01:57] Gonna focus on helping Rob first thouhg [19:02:04] Is there a VE-side commit to address the API breakage? [19:02:11] lets go over it together later today [19:02:25] I didn't know there was any API breakage [19:02:36] it's actually clever that way [19:02:58] i gotta get the girls ready, we are taking bus/bart to SF for lunch, then Mel is taking them [19:03:04] we can meet up for lunch if you want [19:03:13] Yeah that would be cool [19:03:23] Is Melissa up in the city already? [19:05:07] she is on her way back from Santa Clara, but it's actually faster for me, getting to work wise, to do it this way, than to wait for her to get to Pacifica, then pack everyone up, then have her drop me off [19:05:18] Right, yeah [19:05:32] this is one of the very few things she does for herself every year... maybe the only thing [19:05:37] it's a knitting convention [19:05:42] Oh, is this the wool thing? [19:05:45] so, I'm trying my best to make it happen for her [19:05:46] Oh no that one [19:05:51] The wool thing is around her birthday [19:05:52] it's called Stiches West [19:05:56] Right [19:06:05] Lamb and Wool festival is in Dixon, we go as a family [19:06:05] Tell her to say hi to Danese if she's there :) [19:06:16] As you may have heard Danese accepted a job at PayPal recently [19:06:35] Also dang Dixon is far [19:06:50] Now I understand how we got invited to speak at the Davis LUG [19:07:04] i heard, yeah I don't know if Danese is going, she only hits me up when she needs something from me [19:07:22] yes [19:07:23] well done, you've solved it [19:07:43] Only, what, two years late? [19:07:51] More than that actually [19:18:07] (03CR) 10Esanders: "I assumed it was allowed. Check with Trevor." [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114535 (owner: 10Esanders) [19:20:59] RoanKattouw / TrevorParscal / James_F according to this bug report, Parsoid's size shouldn't be set by default,but VE should force it based on the css classes. https://bugzilla.wikimedia.org/show_bug.cgi?id=43336 <-- this would mean I set up rules in DM to force image size to be wiki default despite Parsoid's output. [19:21:17] if the image is set to defaultSize, I mean. [19:22:15] *or* we create dynamic CSS class, but that class would have to depend on which dimension is bigger (so, limit width or limit height) [19:22:19] is it just me, or is this a bit backwards? [19:22:55] mooeypoo: I fail to see how this is relevant, this is complaining about Parsoid not properly respecting size = min(actualSize, defaultSize) [19:23:49] (03CR) 10Jforrester: "Relevant bugs:" [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/108945 (owner: 10Robmoen) [19:23:53] I might be misunderstanding the bug, comment #3 specifically [19:25:17] I wanted to post this as a bug, then found this (and the resolved one) which makes it seem, if I understand it correctly, that we should have VE adjust sizes based on the mw-default-size class. [19:35:47] (03CR) 10Robmoen: Create mixin for node that responds to node click events (033 comments) [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/103062 (owner: 10Robmoen) [19:37:10] rmoen: Are you working on the TOC change right this minute? [19:37:22] RoanKattouw: not atm [19:37:26] OK [19:37:32] Do you have any changes locally that aren't in Gerrit? [19:37:40] Because I'd like to rebase your change [19:37:42] RoanKattouw: Just some debugging [19:37:48] Without screwing you over, ideally [19:38:11] OK; are your debugging changes only against the newly added files [19:38:13] ? [19:38:16] yeah [19:38:21] Alright. sweet [19:38:37] Then you should be able to stash and recover them when I'm done [19:38:41] (03PS7) 10Robmoen: WIP: Table of contents widget [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/108945 [19:38:46] Yay rebase button [19:38:49] Let's see if that worked [19:39:08] RoanKattouw: Rebase button? I'm starting to think I need one [19:39:27] (03CR) 10Jforrester: "Would be good to get this to work for Enter on a selected node, which would fix bug 54827 too." [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/103062 (owner: 10Robmoen) [19:39:52] (03CR) 10jenkins-bot: [V: 04-1] WIP: Table of contents widget [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/108945 (owner: 10Robmoen) [19:40:47] (03PS7) 10Robmoen: Create mixin for ce nodes that responds to dblclick [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/103062 [19:41:15] rmoen: OK that rebase worked, I can now test that code with the latest version of MW core [19:41:18] I also found the first bug [19:41:29] RoanKattouw: i'm so glad [19:41:32] It so happened my test page had an

as the first heading followed by an

[19:41:36] And the resulting behavior is ... fun [19:42:33] I'm gonna start an Etherpad with my findings [19:43:01] RoanKattouw: OK sounds good. pst link when you has [19:43:16] https://etherpad.wikimedia.org/p/VETOC [19:54:13] rmoen: Do you have lunch plans? Because my lunch is gonna be a bit later and I have a 2pm [19:54:34] So if it's convenient for you, I could do some code reading and then we could start a hangout talking about a few things in like 15 minutes-ish? [19:54:45] RoanKattouw: Sure [19:54:48] Awesome [19:55:08] RoanKattouw: Excellent [19:55:31] Then I'll break for lunch whenever Trevor gets here (he's like 30 mins away) and then resume after [19:59:48] (03PS1) 10Ltrlg: Do not add 'required' if not set and false wanted [extensions/TemplateData] - 10https://gerrit.wikimedia.org/r/114770 [20:28:34] rmoen: Dude, you implemented tracking of TOCForce/Disable state, sweet! [20:29:29] RoanKattouw: are you expecting trevor anytime? hey so do you want to have hangout for a few minutes [20:29:47] RoanKattouw: that came out backwards lol [20:29:54] He's on BART right now [20:29:56] With his kids [20:30:00] Because Melissa was in Santa Clara today [20:30:25] So once he gets here we'll go meet them for lunch, Melissa will pick up the kids and he'll come into the office [20:32:28] RoanKattouw: I assume you mean "we" as in ppl in the office. [20:32:38] ;) [20:32:55] Yeah, exclusive we, sorry :) [20:33:07] Apparently there are languages where exclusive we and inclusive we are different words [20:33:11] Which is a useful feature [20:35:28] (03CR) 10Catrope: [C: 04-1] WIP: Table of contents widget (0320 comments) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/108945 (owner: 10Robmoen) [20:35:35] rmoen: OK, I left some comments, leaving for lunch now [20:35:52] Once I'm back I'll take a closer look at the architecture aspects of it all [20:36:02] RoanKattouw: Ok sounds good. [20:36:08] RoanKattouw: Thanks [20:36:15] Re setup/teardown I feel like they should be done in the same place, probably in the widget rather than in the headingnode [20:36:32] But I'll be able to come up with a more intelligent response once I've actually looked at how the whole thing fits together [20:37:07] The overall structure seems pretty sane, have a nested GroupElement and a master widget to track changes and update things [20:37:24] I like the NOTOC/FORCETOC handling and the fact that there's a level 0 item [20:38:05] I think some of the mechanics of how things are tracked and updated exactly might need some work; hopefully we can restructure them in a way that makes it a bit clearer how they work which should help making them less fragile [20:38:53] Sweet dude. I do greatly appreciate the feedback. And I will be going through your comments and will be here when you get back from lunch. [20:38:54] As always, the devil is in the details :) [20:38:58] Anyway, lunch [20:45:10] (03PS1) 10Mooeypoo: Add placeholders to MediaSizeWidget [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/114773 [20:49:14] (03PS4) 10Mooeypoo: [wip] Set up wiki-default image size [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114420 [20:49:50] ok i'm running to Hunter college. Have a good weekend, guys, see ya later! [20:50:30] (03CR) 10jenkins-bot: [V: 04-1] [wip] Set up wiki-default image size [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/114420 (owner: 10Mooeypoo) [21:08:18] (03CR) 10Jhall: [C: 032] [Browser test] @clean the Basic Edit feature [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112659 (owner: 10Hashar) [21:08:25] (03CR) 10jenkins-bot: [V: 04-1] [Browser test] @clean the Basic Edit feature [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112659 (owner: 10Hashar) [21:15:15] rmoen, RoanKattouw: Do you need a room? [21:16:19] James_F: Probably [21:16:24] R32 is free [21:16:51] And I could share it with TrevorP|Away and rdwrer if they like, or they can sit at a desk [21:17:14] I was just going to do it at a desk [21:17:33] RoanKattouw: In that case you probably want a room. [21:17:51] Or we could use my desk if TrevorP|Away is cool with milling about [21:18:13] rdwrer: I think he prefers to sit, so you can steal RoanKattouw's chair now he's gone (hint hint). [21:18:35] As you can see I'm in R33 now that Trevor's back ;) [21:20:40] rdwrer: i'm at my desk now [21:20:42] sorry for the delay [21:20:46] come on down [21:21:14] (03CR) 10Catrope: WIP: Table of contents widget (033 comments) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/108945 (owner: 10Robmoen) [21:21:31] RoanKattouw: I'm just going through your comments, lmk when you want to hangout [21:21:45] rmoen: Moving to R33 now, so I'll invite you to a hangout in a few minutes [21:28:26] (03PS4) 10Hashar: [Browser test] @clean the Basic Edit feature [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112659 [21:29:15] (03CR) 10Jhall: [C: 032] [Browser test] @clean the Basic Edit feature [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112659 (owner: 10Hashar) [21:30:42] (03Merged) 10jenkins-bot: [Browser test] @clean the Basic Edit feature [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112659 (owner: 10Hashar) [22:13:01] Aha [22:13:04] That explains everything [22:13:12] I had the default, original, and large messages [22:13:18] But forgot to rename the last two [22:13:24] So they both overrode "default" [22:13:40] Life makes sense again [22:18:09] rdwrer: :-) [22:27:43] TrevorParscal: How goes? [22:39:31] very close [22:43:50] 20 minutes, eh :) [22:46:12] it's working, just need to get a few final bits done, like automatically popping up above, and closing when you click away [22:46:33] *nod* 'kay [22:57:56] (03PS1) 10Trevor Parscal: Add OO.ui.InlineMenuWidget [oojs/ui] - 10https://gerrit.wikimedia.org/r/114892 [22:58:49] rdwrer: https://gerrit.wikimedia.org/r/#/c/114892/ is the basic widget [22:58:56] it doesn't pop above [22:59:04] and it doesn't close when you click away [22:59:32] The popping above is something I need to add to menu [22:59:43] not the actual InlineMenuWidget (since it's composed) [22:59:48] coming up in a min [23:00:50] well, maybe not a min [23:01:06] Heh. [23:01:08] basically, there are some very complicated things to do with clipping that I need to sort out [23:01:22] I will need to consult with some other guys and come up with a strategy [23:01:30] I wanted to get that patch in so you could start playing with it [23:01:36] Ahh, I see [23:01:37] even though it's not complete [23:01:47] I know you need it to popup above [23:01:54] I get to fuck with my core oojs then, woo :) [23:02:02] :-) [23:02:07] It can pop above or below, I think [23:02:21] well, in the screen you showed me, there wasn't much room below [23:03:02] Yeah, it should probably depend on the scrolltop [23:15:25] TrevorParscal: No addItems? :) [23:18:55] RoanKattouw: if i have a ref to a dm node, is the a method to get the corresponding ce node ? [23:19:15] rmoen: There isn't one, on purpose [23:20:02] A DM can theoretically have multiple CEs [23:20:12] However [23:20:22] RoanKattouw: Oh i see, I should be pulling data from the DM anyways [23:20:26] If you also have a ref to the CE surface or at the very least the CE document/documentNode [23:20:27] I can do withoout [23:20:33] Then you can do an offset-based lookup [23:20:42] Get the offset of the DM node, then look up that offset in the CE document [23:20:47] mooeypoo has done this before and can help you [23:22:01] That's a nice way, though i just realized i was pulling data from the ce node and instead would rather it come from the dm now that i'm building the tree all in one go [23:23:31] RoanKattouw: TLDR, I was being a n00b [23:37:09] RoanKattouw, this may be another dumb question: how do i get the actual text of a VeDmTextNode [23:37:21] like is there an attribute or is it just the html ? attribute ? [23:38:16] like node.getAttribute( 'html' ) ? [23:44:54] (03PS2) 10Trevor Parscal: Add OO.ui.InlineMenuWidget [oojs/ui] - 10https://gerrit.wikimedia.org/r/114892 [23:44:56] (03PS1) 10Trevor Parscal: Basic support for automatically rendering menus above or below [oojs/ui] - 10https://gerrit.wikimedia.org/r/114899 [23:45:07] rdwrer: there you go, that renders it above [23:50:23] rdwrer: you really need this widget though: http://cl.ly/image/3t2x3B241S02 [23:56:33] rdwrer: Would be nice for you to +1 that https://gerrit.wikimedia.org/r/114892 is what you want before it gets merged anyway, BTW. :-) [23:56:39] TrevorParscal: Totally. :-) [23:56:42] 'kay [23:57:46] I dunno about that...it looks too big [23:57:53] Talk to pginer about it if you disagree :) [23:58:35] rdwrer: it says "Hella big" on it, it's a joke [23:58:53] TrevorParscal: "hella" is srs bsnss [23:58:54] although, the use of an inline menu widget for this particular case probably is suspect [23:59:03] I'm not really impressed with the design [23:59:06] at all [23:59:24] but I hope the widget helps you [23:59:26] 2014-02-21 - 15:57:53 Talk to pginer about it if you disagree :)