[00:02:39] Sometimes there is no request made in response to 'review'... [00:03:06] RoanKattouw, Krinkle, whenever you have time, I'd appreciate feedback on https://gerrit.wikimedia.org/r/#/c/110655/ and its dependency [00:05:21] Krenair: No but we always go through a promise I think [00:05:30] So you should be able to hook into that with $.when() etc [00:05:47] jgonera: Sorry, yeah that was in my re-review queue for today, looking [00:06:01] In onSaveDialogReview we usually (when a diff table already exists - this gets cleared only when the user makes a content change) just this.saveDialog.swapPanel( 'review' ); [00:06:11] RoanKattouw, no worries, thanks [00:06:21] jgonera: Also in general, please don't rebase and amend code in the same patchset [00:06:27] Krenair: Aaah [00:06:28] I see [00:06:57] RoanKattouw, I'll try to remember next time [00:07:07] the promises are dealt with in ve.init.mw.Target.prototype - .serialize and .showChanges [00:07:10] (03PS1) 10Jforrester: Update VE core submodule to master (43787a8) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113049 [00:07:23] RoanKattouw, I guess I'll need to make those return the promises and use .then or something? [00:07:29] Krenair: So maybe let's just put this API request in swapPanel() [00:07:37] Under case 'review': [00:07:44] Conditional on the summary actually having changed [00:07:56] And come up with a spinner or whatever to indicate to the user that we're working on the summary preview [00:08:00] ah, that sounds much better [00:08:14] I'd forgotten about the direct switch optimizaton [00:11:42] Hm... We have a spinner class somewhere I think... [00:13:43] jgonera: Hmm, seems fine. I am a bit concerned about the 200ms setTimeout for the toolbar animation, that doesn't seem very generic [00:13:47] But it's part of a larger problem [00:14:11] RoanKattouw, I just moved this code following Krinkle's suggestion, haven't changed anything there [00:14:30] RoanKattouw, we can add a FIXME, but fixing/changing this should not be part of this patch IMHO [00:14:31] Yeah [00:14:34] No it's fine [00:15:14] I was going to file a tech debt bug but BZ is down :S [00:15:29] VE's handling of "wait for this animation to complete" is bad anyway and needs to be refactored [00:15:34] Or, like, actually thought about :) [00:18:58] (03CR) 10Catrope: [C: 032] Move restoreEditSection() to mw.Target [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/111929 (owner: 10JGonera) [00:20:42] (03CR) 10jenkins-bot: [V: 04-1] Move restoreEditSection() to mw.Target [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/111929 (owner: 10JGonera) [00:22:17] (03PS3) 10Alex Monk: Show preview of edit summary in review screen [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112490 [00:22:25] RoanKattouw, that function is just what I was looking for the other day, thanks [00:22:35] (03PS3) 10JGonera: Move restoreEditSection() to mw.Target [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/111929 [00:22:56] (03PS5) 10JGonera: Make MobileViewTarget scroll to desired section [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/110655 [00:23:57] TrevorParscal, re: using the store for image dimensions [00:24:41] there's no real reason to use the IVStore other than it's global to the document [00:25:50] so it was an improvement over the previous method of using a local object in the node [00:26:16] (03CR) 10Kaldari: [C: 031] Make MobileViewTarget scroll to desired section [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/110655 (owner: 10JGonera) [00:26:18] (03CR) 10Catrope: [C: 032] Move restoreEditSection() to mw.Target [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/111929 (owner: 10JGonera) [00:26:46] (03CR) 10Catrope: [C: 032] Make MobileViewTarget scroll to desired section [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/110655 (owner: 10JGonera) [00:26:50] I believe getImageInfo is the only place that is used, so you won't break anything [00:27:19] Would love to get a follow-up review on https://gerrit.wikimedia.org/r/#/c/110655/ since another patch is waiting on it in MobileFrontend. [00:27:38] (03Merged) 10jenkins-bot: Move restoreEditSection() to mw.Target [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/111929 (owner: 10JGonera) [00:29:04] (03Merged) 10jenkins-bot: Make MobileViewTarget scroll to desired section [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/110655 (owner: 10JGonera) [00:29:35] (03CR) 10Catrope: [C: 032] Update VE core submodule to master (43787a8) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113049 (owner: 10Jforrester) [00:31:15] (03Merged) 10jenkins-bot: Update VE core submodule to master (43787a8) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113049 (owner: 10Jforrester) [00:32:35] (03CR) 10Catrope: [C: 04-1] Show preview of edit summary in review screen (032 comments) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112490 (owner: 10Alex Monk) [00:33:33] RoanKattouw, yeah was about to give up and ask about why my loading thing isn't working [00:34:02] Well it's hard to debug [00:34:13] I did a 5-second (and then a 50-second) setTimeout in the done handler [00:34:24] Then inspected and looked at what styles came along with -working [00:34:32] (03PS16) 10Jhall: [Browser test] Headless browser test(s) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/111890 [00:34:36] When I unchecked display:none; , I saw the spinner come up on the right-hand side [00:37:13] RoanKattouw, to check whether the summary wikitext has changed, I guess we could store it using .data()? [00:37:42] Just store it in this.someProperty [00:39:37] k [00:40:41] James_F, what kind of spinner do we want on this (Show preview of edit summary in review screen) while it's loading? [00:41:41] Krenair: Not sure; something familiar, maybe? [00:41:57] Krenair: But the big horizontal one seems overly "heavy". [00:42:15] I was thinking the normal VE loading ... thing. I actually don't really know how to describe it :| [00:43:06] Horizontal loading GIF of doom? :-) [00:44:06] I guess so. [00:44:24] I think MW has a circular one somewhere... Or maybe that's just LQT. [00:44:46] I think that's mediawiki.spinner.js. [00:44:47] I think. [00:44:52] Krenair: James_F: jquery.spinner [00:45:02] MatmaRex: Thanks! [00:45:24] (03CR) 10Catrope: [C: 04-1] "Code looks good and it generally works well. One thing, functionality-wise: you can press Ctrl+S in a non-save panel (like the review pane" (031 comment) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/110434 (owner: 10Alex Monk) [00:45:33] Seems I was thinking of the mw-ajax-loader class [00:50:31] (03CR) 10Esanders: "Style-wise there is nothing distinguishing the label ('Edit summary preview') from the value here. It also looks a little weird having a d" [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112490 (owner: 10Alex Monk) [00:55:33] Krenair / edsanders: You could re-use the "Preview of edit summary" from EditPage.php… [01:01:43] (03CR) 10Jforrester: "Fine with Ctrl+S working in the review-changes panel (though yes, it should be disabled in the "you have an edit conflict, oh well" screen" [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/110434 (owner: 10Alex Monk) [01:03:07] (03CR) 10Catrope: "No, it doesn't appear there because it's not a shortcut (Ctrl+S) but an access key (Alt+S) (sorry, I misspoke earlier when I talked about " [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/110434 (owner: 10Alex Monk) [01:04:22] RoanKattouw / Krenair: I think it should appear in the list; it's a pretty key ability. [01:04:28] (03PS4) 10Alex Monk: Show preview of edit summary in review screen [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112490 [01:05:32] James_F: Well sure, but in that case maybe it should also be bound to Ctrl+S ? [01:05:48] Because Ctrl+ = VE keyboard shortcut, Alt+ = MW accesskey [01:06:27] (03CR) 10jenkins-bot: [V: 04-1] Show preview of edit summary in review screen [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112490 (owner: 10Alex Monk) [01:06:36] RoanKattouw: Binding to Ctrl+S is a bit evil; that's a standard short-cut. [01:06:41] Yes [01:06:48] RoanKattouw: Maybe do both the accesskey and the short-cut? [01:06:56] (Ctrl+Shift+S perhaps?) [01:07:04] Right, that's what I was saying, do both and only document the shortcut [01:07:06] That way it's discoverable and consistent. [01:07:08] Yeah. [01:07:13] Because the shortcuts system is self-documenting [01:07:25] (Consistent with the wikitext editor.) [01:07:29] Krenair: Does that work for you? [01:08:13] (03CR) 10Catrope: [C: 032] Explain what the number on the edit summary screen means [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112318 (owner: 10Alex Monk) [01:08:49] James_F, so we're overriding Ctrl+S as well? [01:09:09] Or adding Ctrl+Shift+S? [01:09:20] Krenair: No, not over-riding Ctrl+S. Just adding Ctrl+Shift+S. [01:09:34] (03Merged) 10jenkins-bot: Explain what the number on the edit summary screen means [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112318 (owner: 10Alex Monk) [01:10:48] RoanKattouw, VE has a system for doing this? [01:11:03] (03CR) 10Catrope: [C: 04-1] Show preview of edit summary in review screen (032 comments) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112490 (owner: 10Alex Monk) [01:11:10] Krenair: For Ctrl+ stuff yes [01:11:12] Let me find it [01:11:40] ve.ui.TriggerRegistry.js line 66 and below [01:11:44] I'm still waiting for Jenkins to tell me what was wrong with that changeset it just Verified-1'd [01:11:53] Right now the system is excessively magical in that every trigger corresponds to an Action with the same name [01:12:10] Krenair: The Jenkins web server appears to be broken but I went through and told you what was wrong in inline comments [01:12:20] (I mean, I didn't rerun the jobs locally, I just eyeballed it) [01:15:38] Krenair: The trigger system is not too well-documented and it should be improved (it's a bit too magical right now and you can't have multiple shortcuts for the same action cleanly) but if you chase down references for an easy-to-grep trigger name like 'heading5', 'commandHelp' or 'pasteSpecial' you should be able to figure out how it all fits together [01:16:38] (03PS5) 10Alex Monk: Show preview of edit summary in review screen [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112490 [01:18:58] (03CR) 10Catrope: [C: 04-1] "Apart from the minor comment inline, the implementation looks good to me. I'll ask James and Trevor to comment on the design issue that Ed" (031 comment) [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112490 (owner: 10Alex Monk) [01:19:53] (03PS6) 10Alex Monk: Show preview of edit summary in review screen [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112490 [01:35:06] (03CR) 10Catrope: [C: 04-1] "I ran the appearance by James and he said it looked OK (not optimal but we can revisit later)." [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112490 (owner: 10Alex Monk) [01:35:11] Krenair: Final -1 :) [01:35:20] (Sorry for all the back and forth on that one) [01:35:44] it's ok, probably half of it is my fault anyway [01:44:02] (03PS1) 10Trevor Parscal: Make disabled state inheritable [oojs/ui] - 10https://gerrit.wikimedia.org/r/113068 [01:44:09] (03PS1) 10Trevor Parscal: Removed some of the drop-shadows on buttons [oojs/ui] - 10https://gerrit.wikimedia.org/r/113069 [01:46:26] (03PS2) 10Catrope: Make disabled state inheritable [oojs/ui] - 10https://gerrit.wikimedia.org/r/113068 (owner: 10Trevor Parscal) [01:46:35] (03CR) 10Trevor Parscal: "Needs I02b933d1d51d10aa4fbd2e7ab3ba3e71d260a2f1 to work properly." [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112041 (owner: 10Mooeypoo) [01:48:56] RoanKattouw, ugh, the new classes mess up the loading animation [01:50:32] Hrmph [01:50:59] Maybe introduce another wrapper div and put the loading class on that one? [01:51:07] (I guess this is how people end up with div soup) [01:52:43] RoanKattouw, ugh... I would just use $( 'div.mw-summary-preview' ) but it's in an iframe [01:53:15] (03PS3) 10Catrope: Make disabled state inheritable [oojs/ui] - 10https://gerrit.wikimedia.org/r/113068 (owner: 10Trevor Parscal) [01:53:29] Use that from where? [01:53:45] swapPanel [01:54:02] Do you know about .find() ? [01:54:14] You can do $outerDiv.find( '.mw-summary-preview' ) [01:54:37] Or just save a reference to the mw-summary-preview div, that's probably best [01:54:52] Yes, though I felt $( 'div.mw-summary-preview' ) was simpler and wanted to use that instead [01:54:54] If you created something, you have no excuse for then having to find it back later with a selector, you should just retain a reference to it [01:54:59] Right, yeah that doesn't work [01:55:08] this.$( ' ...' ) would work, but as I said, selecting is discouraged [01:56:31] (03CR) 10Catrope: [C: 032] Make disabled state inheritable [oojs/ui] - 10https://gerrit.wikimedia.org/r/113068 (owner: 10Trevor Parscal) [01:56:37] (03PS2) 10Trevor Parscal: Removed some of the drop-shadows on buttons [oojs/ui] - 10https://gerrit.wikimedia.org/r/113069 [01:57:04] RoanKattouw, oh, this.$, I knew I'd seen something else deal with it like this before [01:57:21] (03Merged) 10jenkins-bot: Make disabled state inheritable [oojs/ui] - 10https://gerrit.wikimedia.org/r/113068 (owner: 10Trevor Parscal) [01:57:22] Anyway I just tried that and it still doesn't display quite right :/ [01:58:08] (03PS3) 10Catrope: Removed some of the drop-shadows on buttons [oojs/ui] - 10https://gerrit.wikimedia.org/r/113069 (owner: 10Trevor Parscal) [01:58:48] (03CR) 10Catrope: [C: 04-1] "Trevor said something about background transitions that need to be pulled out, and changes not mentioned in the commit summary (about maki" [oojs/ui] - 10https://gerrit.wikimedia.org/r/113069 (owner: 10Trevor Parscal) [01:59:31] :S [01:59:34] Well I'm about to go [01:59:49] But you can ask Trevor to help debug that for you tomorrow, or ask me on Friday (I'm out tomorrow) [02:00:37] I'll leave a PS in gerrit which has the issue [02:01:20] (03PS7) 10Alex Monk: Show preview of edit summary in review screen [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112490 [02:01:55] (03CR) 10Alex Monk: [C: 04-1] "Unfortunately now the loading icon doesn't show properly" [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112490 (owner: 10Alex Monk) [13:28:08] (03PS1) 10Esanders: Move commands into static getter so they can be overridden. [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/113116 [13:40:02] (03PS1) 10Esanders: Add MW-specific keyboard shortcut help. [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113118 [13:40:13] (03CR) 10jenkins-bot: [V: 04-1] Add MW-specific keyboard shortcut help. [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113118 (owner: 10Esanders) [15:17:54] (03PS1) 10Hashar: [Browser test] bump mediawiki_selenium to 0.2.3 [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113127 [15:19:09] (03CR) 10Hashar: [C: 031] "mediawiki_selenium 0.2.3 let us specify the destination of screenshots via SCREENSHOT_FAILURES_PATH env variable." [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113127 (owner: 10Hashar) [15:22:15] (03PS2) 10Hashar: [Browser test] bump mediawiki_selenium to 0.2.3 [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113127 [17:46:41] (03CR) 10Jhall: [C: 032] [Browser test] bump mediawiki_selenium to 0.2.3 [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113127 (owner: 10Hashar) [17:48:05] (03Merged) 10jenkins-bot: [Browser test] bump mediawiki_selenium to 0.2.3 [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113127 (owner: 10Hashar) [18:36:09] (03PS1) 10Mooeypoo: Fix SelectWidget's selectItem method [oojs/ui] - 10https://gerrit.wikimedia.org/r/113162 [18:52:15] James_F, should this be resolved is "WONTFIX" or "INVALID" ?or... stay open 'till there's a way to work on inline images? https://bugzilla.wikimedia.org/show_bug.cgi?id=61153 [18:55:37] mooeypoo: Keep open until then? [18:57:17] until we can work on inline images? [18:57:22] Yeah. [18:57:25] it's intended behavior, we just can't edit it. [18:57:28] ok, np. [18:57:31] Which hopefully won't be too far away? [18:57:40] yep [18:57:49] Yeah, but it means we'll want to adjust the behaviour of the dialog to make it clear what options go with what other options. [18:58:04] yeah we might have to do a whole tutorial if that's the case :\ [18:58:25] btw, that reminds me... do you guys think we should have a [?] buttons ? [18:58:54] Extra information about certain tools or options, with some tooltip or something similar? [18:59:08] I think some of those options (Especially media edit, but some page config options too) could use it [18:59:39] It's a possibility, yeah. [19:00:05] Design have talked about it, but don't yet have a suggestion for how the additional help should appear. [19:00:14] Maybe we'll give the task to you. :-) [19:00:52] Sure :) [19:01:34] Krenair: Hmm; in testing on Beta Labs and on MW.org the disambig list never seems to get populated… [19:02:05] James_F, hm, there's another behavior bug in MediaSizeWidget .. not sure if it's there or in scalable. I might have to do another hack to fix it until the whole superawesome media datamodel is done [19:02:16] mooeypoo: Fun. :-( [19:02:26] It's not limiting maxDimensions [19:02:50] you know if ed's going to be around today? [19:03:07] It might be best for me to consult with him on this before I mess around with his scalable stuff [19:03:52] TrevorP|Away: http://fab.wmflabs.org/project/board/1/ [19:03:56] mooeypoo: Later, yes. [19:04:51] James_F, that's weird... it was working on my machine [19:04:52] and there are a bunch of things that are a bit scattered all over the place. The fix I made for 'defaultSize', apparently it's already in MWResizableNode, only it didn't apply to the images. I'm fixing up things but it's a bit hacky.. then again, the entire thing is going to be refactored anyways soon... [19:06:14] James_F, what I'm trying to ask, diplomatically, is, should I bother applying hacks, or wait for ed and trust the new refactor to be coming soon? [19:06:19] * James_F grins. [19:07:15] mooeypoo: I'm still working on the MWImageModel, but it's getting close - hoping to finish today [19:07:25] it should centralize all the business logic of images [19:07:33] TrevorParscal, yeah, your work will fix most (if not all) of this [19:07:42] so I don't know if I should waste time on fixing these bugs individually [19:07:46] then, we shouldn't have to be scattering this crap around, and can probably fix it properly in one place [19:07:51] * mooeypoo nods [19:08:16] James_F, um, you sure that change got deployed to MW.org yet? [19:08:16] it's not a waste, I'm pulling code in from all these places and centralizing it, then solinvg issues as I go [19:08:39] TrevorParscal, most of those problems are *either* bugs in MediaSizeWidget *or* Scalable, and the hard thing for me is to figure out which to fix up... usually I'd say this probably requires a new refactor, but that's exactly what you're doing [19:09:13] Krenair: Apparently not. But it also doesn't work on Beta Labs. [19:09:28] Krenair: Though right now I can't route to Beta Labs at all, so… [19:09:43] TrevorParscal, I feel like I'm working on bandades while you're transferring the patient's consciousness to a new body. [19:09:49] lol [19:09:59] mooeypoo: Band-Aids? [19:10:12] i'm sorry that I'm in the middle of this, let me focus on this model thing and we can talk about this again at that point [19:10:21] James_F, oh, is that how you spell it? Also, in this analogy, *that* is what bothered you? :D [19:10:22] James_F: you know, small self-adhesive bandages [19:10:28] it's a brand name [19:10:30] mooeypoo: :-D [19:10:32] Yup. [19:10:42] lol [19:11:16] James_F, WMF on http://en.wikipedia.beta.wmflabs.org/ [19:11:18] WFM* even [19:12:18] (03PS4) 10Trevor Parscal: Removed some of the drop-shadows on buttons [oojs/ui] - 10https://gerrit.wikimedia.org/r/113069 [19:12:29] (03PS5) 10Trevor Parscal: Removed some of the drop-shadows on buttons [oojs/ui] - 10https://gerrit.wikimedia.org/r/113069 [19:13:50] (03CR) 10Jforrester: [C: 032] Removed some of the drop-shadows on buttons [oojs/ui] - 10https://gerrit.wikimedia.org/r/113069 (owner: 10Trevor Parscal) [19:14:11] (03Merged) 10jenkins-bot: Removed some of the drop-shadows on buttons [oojs/ui] - 10https://gerrit.wikimedia.org/r/113069 (owner: 10Trevor Parscal) [19:14:14] TrevorParscal, you're not at all in the middle, I just want to see if I should concentrate on these bugs at all atm, or bug James about a new task. [19:24:03] * James_F grins. [19:38:25] (03PS1) 10Trevor Parscal: [WIP] Centralize image handling into a stand-alone model [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113163 [19:39:43] (03CR) 10jenkins-bot: [V: 04-1] [WIP] Centralize image handling into a stand-alone model [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113163 (owner: 10Trevor Parscal) [19:44:27] er... I am writing a new page with VE, and after trying to paste something, the entire page is giving me endless errors... and now it won't save, and won't copy, so I have to start from scratch :( [19:45:13] mooeypoo: :-( [19:45:25] mooeypoo: Can you repeat it? If so, edsanders|away would love your bug. [19:45:26] "Uncaught TypeError: Cannot call method 'getComparableObjectForSerialization' of undefined " [19:45:38] well. I'm trying to salvage a pretty long page first [19:45:43] * James_F nods. [19:45:45] Meh. [19:45:52] Dump to HTML file. [19:46:01] (Browser > File > Save page) [19:46:12] Then open locally, and copy-and-paste into a new VE instance. [19:46:24] that's the error that started it all... after that, links started crashing, and I get 'ncaught TypeError: Cannot call method 'isFocusable' of null' on every click [19:46:38] Oh dear. [19:46:55] Yeah, that's one of the big problems with large JS programs. [19:48:07] ah! save + copy + paste into sublime = works [19:48:12] so at least it's salvaged :P [19:48:20] now I'll try again and see if I can replicate the bug [19:53:07] There we go. I replicated. [19:54:35] James_F, what category does copy/paste get into in bugzilla? [19:54:42] mooeypoo: CE. [20:00:21] James_F, https://bugzilla.wikimedia.org/show_bug.cgi?id=61332 [20:08:37] mooeypoo: Thanks. [20:10:43] mooeypoo: are you already aware of certain situations (haven't figured out the details yet) where the resize label remains display: block but 0 opacity? [20:11:08] I've noticed because it's incorrectly positioned after making a change nearby, and blocking clicks [20:11:55] I can reproduce it, if you need instructions [20:16:02] TrevorParscal, you mean on the image itself? I don't mind taking a look, but I didn't work on the resizable/scalable piece, that was ed. [20:16:52] But if you tell me where to reproduce, I can fix it up [20:17:00] now I can't reproduce... arg [20:17:13] if I can get it again I will let you know [20:17:46] * mooeypoo nods [20:18:04] I'm writing a complete-newbie-guide to VE development [20:18:27] well.. right now it's more of a "how to set up your system without visiting 10 different tutorials!" guide [20:18:38] cool [20:18:45] that's really awesome [20:18:48] (03PS1) 10Ltrlg: Updated specs example to use new types [extensions/TemplateData] - 10https://gerrit.wikimedia.org/r/113173 [20:18:51] um, https://gerrit.wikimedia.org/r/#/c/113162 [20:19:07] so, I don't really see why this is any different [20:19:34] I mean, the guts of initializeSelection precalculate "selected" but otherwise the only difference is that it sets both selection and highlight [20:19:41] which, selectItem shouldn't do [20:19:51] TrevorParscal, two things, first, I thought there was no need to repeat code at all since it seems the only difference between the two methods other than the event [20:19:53] because the selection and highlight are not the same [20:20:21] another issue is that it didn't seem to reset previous selections, while the initialize does [20:20:33] the way it resets is the same [20:20:37] TrevorParscal, wait, what's the difference in this case? A selected item should be highlighted? [20:20:55] it uses setSelected( this item === the item we are looping through ) [20:21:09] highlight is like the mouse hovering or the keyboard navigating [20:21:40] TrevorParscal, I can't manage to replicate atm *but* this started popping up an error of too much recursion [20:21:42] selecting is a separate mutually exclusive property [20:22:09] After you selectItem() once or twice, unless you initializeSelection() on each .setup of the dialog [20:22:39] interesting [20:22:40] TrevorParscal, but wait, a selected item should be highlighted, no? [20:22:57] in this case, I mean, because we only have 1-at-a-time selection [20:23:26] not nessecarily, I mean at the time the user is selecting it themselves, it will natrually be highlighted as well, but if one item is selected and the user navigates to another to select it instead, there is a time where these diverge [20:23:46] selected is shown as a check icon usually, while highlighted is a change of color [20:24:09] I may have misunderstood, but the 'initializeSelection' seems to be doing exactly what we want to do when we choose an item in the one-at-a-time selector, except that we also want it to emit 'change'... no? [20:24:23] think of selected as the model state, and highlighted as the view state, and running setSelected is the controller taking action on input [20:24:41] hm [20:26:09] TrevorParscal, ok, but the difference between 'selectItem' and initializeSelection( item ) seemed to be more than just the event and then highlight [20:26:16] initializeSelection is used to override the MVC loop, to change the view in a way that makes it look as if the model changed, without changing the model [20:26:47] mooeypoo: there's no difference, the loops and methods of changing state are identical [20:27:06] wait, I just noticed that, I got confused for a minute with selected, but that's local [20:27:15] yes [20:27:24] exactly, it's the same thing, just cached since we use it twice [20:27:28] so.. why did I get endless recursion. I was pretty sure it was failing over multiple selected items, but apparently that wasn't it [20:27:56] especially since calling .initializeSelection() on every .setup fixed it. [20:27:59] hm. [20:28:08] my point is this, one - we shouldn't set both selection and highlight in this case (and maybe not in the case of initializeSelection, perhaps there should be initializeHighlight as a separate method) [20:28:21] * mooeypoo nods [20:28:24] two - this doesn't really change how the mutually exclusive thing works [20:28:38] because it uses the same exact approach inside intializeSelection [20:28:40] I thought they are both the same, only one from the object perspective and the other from the view perspective [20:28:44] but apparently I was wrong. [20:28:49] no worries [20:28:56] * mooeypoo nods [20:29:02] what is the exact case where you are seeing trouble by the way? [20:29:11] yeah I'm trying to replicate now and I can't manage to [20:29:29] there were a couple of fixes in between me seeing this error and now, so it *may* have been an unrelated thing that I accidentally fixed [20:31:06] (03CR) 10Trevor Parscal: [C: 04-1] "This doesn't quite fix the problem - let's hold off on this until we can get to the bottom of it." [oojs/ui] - 10https://gerrit.wikimedia.org/r/113162 (owner: 10Mooeypoo) [20:31:49] brb - lunch [20:31:56] bon apetit [20:39:56] TrevorParscal, geez, now I'm intentionally trying to break things and it's still working! [20:58:25] (03PS1) 10Jhall: [Browser test] Basic maintenance for headings browser test. [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113179 [21:11:04] (03CR) 10Cmcmahon: [C: 032] "robustification" [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113179 (owner: 10Jhall) [21:12:11] (03Merged) 10jenkins-bot: [Browser test] Basic maintenance for headings browser test. [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113179 (owner: 10Jhall) [21:13:44] (03PS1) 10Ltrlg: Make TemplateDataGenerator use new types [extensions/TemplateData] - 10https://gerrit.wikimedia.org/r/113183 [21:13:46] (03CR) 10jenkins-bot: [V: 04-1] Make TemplateDataGenerator use new types [extensions/TemplateData] - 10https://gerrit.wikimedia.org/r/113183 (owner: 10Ltrlg) [21:16:52] (03PS2) 10Ltrlg: Make TemplateDataGenerator use new types [extensions/TemplateData] - 10https://gerrit.wikimedia.org/r/113183 [21:35:43] (03PS1) 10Trevor Parscal: Only allow pointer events on shields inside generated content nodes [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/113256 [21:38:38] (03PS1) 10Trevor Parscal: Prevent clicks on top-most shield for centered image nodes [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113258 [21:39:13] (03CR) 10jenkins-bot: [V: 04-1] Prevent clicks on top-most shield for centered image nodes [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113258 (owner: 10Trevor Parscal) [21:41:08] (03PS2) 10Trevor Parscal: Prevent clicks on top-most shield for centered image nodes [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113258 [21:41:28] (03PS2) 10Trevor Parscal: Only allow pointer events on shields inside generated content nodes [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/113256 [21:41:39] (03CR) 10jenkins-bot: [V: 04-1] Prevent clicks on top-most shield for centered image nodes [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/113258 (owner: 10Trevor Parscal) [21:42:34] (03CR) 10Trevor Parscal: "Timo, can you please fix the build script then?" [oojs/ui] - 10https://gerrit.wikimedia.org/r/112790 (owner: 10Trevor Parscal) [21:46:13] James_F, can you replicate the issue described in https://bugzilla.wikimedia.org/show_bug.cgi?id=61333 ? [21:49:58] (03CR) 10Jforrester: [C: 032] "Let's fix it in post." [oojs/ui] - 10https://gerrit.wikimedia.org/r/112790 (owner: 10Trevor Parscal) [21:50:24] (03Merged) 10jenkins-bot: Moved PNG icons to their own variant [oojs/ui] - 10https://gerrit.wikimedia.org/r/112790 (owner: 10Trevor Parscal) [21:51:06] James_F, first draft: https://www.mediawiki.org/wiki/User:Mooeypoo/VisualEditor_Development [21:53:31] TrevorParscal, mooeypoo: What is the intended scope of the ViewPageTarget css files? Since "viewPageTarget" seems to include both the content being edited and the VE toolbar, I'm not sure what it is supposed to be excluding, if anything. Is "viewPageTarget" just an alias for "VisualEditor" or does it mean something more specific? [21:54:03] ok [21:54:17] a target is a place in which VE is being instantiated [21:54:48] could be anywhere, maybe you are using VE in flow, you might have FlowPostTarget or something [21:55:52] the idea is that when you actually instantiate VE in various places, you will need totally different setup/loading/teardown procedures [21:55:58] you will get the data from different places [21:56:10] Ah, so "ViewPageTarget" is like a file name namespace, i.e. what comes after it is the context of the styling [21:56:14] you will render the toolbar differently, or not at all, or in a different place [21:56:20] you will have a different method of saving, etc. [21:56:57] yeah, we call it ViewPageTarget because it's VE being instantated on an article view action page [21:57:30] got it [21:58:08] I was thinking maybe it was some kind of object in and of itself [21:58:57] it's possible there are things we've baked into viewpagetarget that might be useful elsewhere, and also that there are things we've left in init.Target that are desktop or even viewpage specific, so some shuffling might need to be done as we build out more targets [21:59:55] kaldari, similarly, we have MobileViewTarget to set up VE on mobile. And I moved one method from ViewPageTarget to Target because it can be reused in MobileViewTarget [22:00:08] TrevorParscal: actually I was thinking about moving some of the oo-ui styles that are Vector specific into the viewPageTarget-vector file [22:00:26] awesome, that will be good [22:01:01] just wanted to makes sure I wasn't moving them to the wrong place [22:01:05] TrevorParscal: https://gerrit.wikimedia.org/r/#/c/113264/ is the last OOjs UI update for core for a week, I think – if that makes sense/ [22:04:42] kaldari: I think you are on the right track [22:34:22] Krinkle: Heya. :-) [22:36:10] TIMO! [22:36:28] how is the city of saint diego? [22:36:34] Nice [22:36:58] Attaching for #jqcon though [22:37:08] * James_F nods. [22:41:12] (03PS1) 10Trevor Parscal: Actually hide the size label when not in use [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/113280 [22:45:07] (03CR) 10Trevor Parscal: [C: 032] Let users set #REDIRECT and __STATICREDIRECT__ status [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/95213 (owner: 10Jforrester) [22:45:21] (03PS1) 10Jforrester: Update OOjs UI to v0.1.0-pre (7788dc6700) [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/113282 [22:46:17] (03CR) 10jenkins-bot: [V: 04-1] Update OOjs UI to v0.1.0-pre (7788dc6700) [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/113282 (owner: 10Jforrester) [22:46:35] (03CR) 10jenkins-bot: [V: 04-1] Let users set #REDIRECT and __STATICREDIRECT__ status [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/95213 (owner: 10Jforrester) [22:51:25] (03CR) 10Trevor Parscal: [C: 032] Let users set #REDIRECT and __STATICREDIRECT__ status [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/95213 (owner: 10Jforrester) [22:51:29] (03PS2) 10Jforrester: Add icons demo [oojs/ui] - 10https://gerrit.wikimedia.org/r/112789 (owner: 10Trevor Parscal) [22:51:45] (03CR) 10Jforrester: [C: 032] Add icons demo [oojs/ui] - 10https://gerrit.wikimedia.org/r/112789 (owner: 10Trevor Parscal) [22:52:21] (03Merged) 10jenkins-bot: Add icons demo [oojs/ui] - 10https://gerrit.wikimedia.org/r/112789 (owner: 10Trevor Parscal) [22:52:53] (03CR) 10jenkins-bot: [V: 04-1] Let users set #REDIRECT and __STATICREDIRECT__ status [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/95213 (owner: 10Jforrester) [22:53:15] (03PS2) 10Trevor Parscal: Actually hide the size label when not in use [VisualEditor/VisualEditor] - 10https://gerrit.wikimedia.org/r/113280 [22:59:32] James_F, hi, did you see my message earlier? [23:00:45] Krenair: No? [23:01:39] James_F, oh ok. Can you replicate the issue described in https://bugzilla.wikimedia.org/show_bug.cgi?id=61333 ? [23:03:10] Krenair: Hmm. No. ryasmeen – do you think it might have been another faulty gadget? [23:04:07] Ok let me just try reproducing it disabling all of the active gadgets [23:08:41] (03PS1) 10Krinkle: build: Concatenation task should not include 'default' in a variant [oojs/ui] - 10https://gerrit.wikimedia.org/r/113288 [23:09:07] (03CR) 10Krinkle: "Follow-up I5b9ae4a8b1a42ac7e6f4142aff2d48daf782cc1c" [oojs/ui] - 10https://gerrit.wikimedia.org/r/112790 (owner: 10Trevor Parscal) [23:09:25] Trevor: Kaity has suggested that the generic version of oojs-ui drop most of the rounded corners (except on buttons) and that these be implemented in the Vector-specific styles instead. Does that seem reasonable? [23:10:02] (03CR) 10Trevor Parscal: [C: 032] build: Concatenation task should not include 'default' in a variant [oojs/ui] - 10https://gerrit.wikimedia.org/r/113288 (owner: 10Krinkle) [23:10:03] FWIW, that will also make it look less awkard in monobook [23:10:16] (03PS2) 10Krinkle: build: Concatenation task should not include 'default' in a variant [oojs/ui] - 10https://gerrit.wikimedia.org/r/113288 [23:10:16] kaldari: You mean, less good? :-) [23:10:30] basically, yes [23:11:08] I don't think we hate Monobook users that much, do we? [23:15:12] (03CR) 10Trevor Parscal: [C: 032] build: Concatenation task should not include 'default' in a variant [oojs/ui] - 10https://gerrit.wikimedia.org/r/113288 (owner: 10Krinkle) [23:15:30] (03CR) 10Trevor Parscal: [C: 032] Let users set #REDIRECT and __STATICREDIRECT__ status [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/95213 (owner: 10Jforrester) [23:15:39] (03Merged) 10jenkins-bot: build: Concatenation task should not include 'default' in a variant [oojs/ui] - 10https://gerrit.wikimedia.org/r/113288 (owner: 10Krinkle) [23:16:53] (03CR) 10jenkins-bot: [V: 04-1] Let users set #REDIRECT and __STATICREDIRECT__ status [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/95213 (owner: 10Jforrester) [23:33:21] (03CR) 10Trevor Parscal: "When you check the box to set a position, a default choice (or perhaps the previous choice before unchecking the box) should be used. Righ" [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112041 (owner: 10Mooeypoo) [23:33:26] (03CR) 10Trevor Parscal: [C: 04-1] Revamp media edit dialog's position widget [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112041 (owner: 10Mooeypoo) [23:35:00] (03CR) 10Trevor Parscal: [C: 032] core: Add a 'super' property to inheriting classes [oojs/core] - 10https://gerrit.wikimedia.org/r/112148 (owner: 10Krinkle) [23:35:56] (03Merged) 10jenkins-bot: core: Add a 'super' property to inheriting classes [oojs/core] - 10https://gerrit.wikimedia.org/r/112148 (owner: 10Krinkle) [23:39:33] ryasmeen, still there? [23:40:14] (03CR) 10Trevor Parscal: [C: 04-1] "Maybe we could somehow avoid this triplication? Couldn't they go in ve.init.mw.ViewPageTarget.css without ending up in ve.init.mw.MobileVi" [extensions/VisualEditor] - 10https://gerrit.wikimedia.org/r/112618 (owner: 10Kaldari) [23:40:34] Hey Krenair, yes I tried reproducing disabling all gadgets I had active in my preferences.Couldn't reproduce after that. [23:42:14] ryasmeen, what gadgets did you have enabled? [23:45:10] Krenair, Citation expander,Reference tooltips and GoogleTrans [23:47:25] To be exact, its actually citation expander which is causing this error [23:47:36] Okay, this is caused by the citation expander [23:47:40] yep, just got that bit [23:59:37] ryasmeen, it looks like this has been brought up on the talk page for the gadget code: https://en.wikipedia.org/wiki/MediaWiki_talk:Gadget-citations.js [23:59:52] "This script puts bogus values inside event handler attributes of edit form buttons, raising an exception whenever either of them is clicked. This can be very annoying when developing other scripts"