| mattuk1972 | hey frankban: | 11:35 |
|---|---|---|
| mattuk1972 | Ive got a fair bit of feedback -all small little details - would you like me to talk you through it or make a document? | 11:35 |
| frankban | mattuk1972: some notes would be great | 11:36 |
| mattuk1972 | okily | 11:36 |
| frankban | mattuk1972: thanks | 11:36 |
| bac | frankban: regarding colors, I used a color picker to look at the colors on the assets referenced on the visual design | 11:58 |
| bac | they are similar but a bit different, which is why i mentioned it | 11:59 |
| frankban | bac: ah! i have to re-check then... however, I am also waiting for the UX review, so I will fix things up all together. thanks Brad. | 12:01 |
| bac | frankban: we've got too many specs to reference! | 12:01 |
| frankban | bac: heh... and the borders in charm-store_detailview_preview.png are shadowed, that can explain the color picking mismatches. | 12:10 |
| hazmat | frankban, pushed test fixes for rapi fwiw | 12:56 |
| frankban | hazmat: cool | 12:58 |
| mattuk1972 | Frankban: bac: was just going to work though the screen grab and mark out the difference in colour,font weight size and colour and cell size - theres quite a lot of difference | 12:59 |
| bac | hazmat: welcome back. you here for real? | 13:00 |
| frankban | mattuk1972: thanks | 13:00 |
| hazmat | bac, not yet, just dropping by, and going through emails | 13:01 |
| hazmat | bac, back for real next week | 13:01 |
| === gary_poster|away is now known as gary_poster | ||
| gary_poster | There is WIP room | 13:19 |
| gary_poster | looking to give reviews... | 13:19 |
| frankban | mattuk1972: awesome review, thanks! | 13:42 |
| gary_poster | frankban, approved your branch with chatty comments | 13:59 |
| frankban | cool gary_poster, thanks. | 14:04 |
| gary_poster | np | 14:15 |
| gary_poster | Makyo, hey. Should I mark https://code.launchpad.net/~makyo/juju-gui/replace-service-module/+merge/132938 as rejected so we get it out of the review list? | 14:19 |
| gary_poster | Also, I think you said you had tests fixed for https://codereview.appspot.com/6782063/ . If you push those then I can review. I'm OK with separating out the subordinate creation problems into a separate card | 14:21 |
| gary_poster | bac, I dunno why LP didn't recognize https://code.launchpad.net/~bac/juju-gui/trunk/+merge/133060 as merged, but it is, so I am marking it as such | 14:22 |
| BradCrittenden | thanks gary | 14:23 |
| === BradCrittenden is now known as bac | ||
| gary_poster | benji, tveronezi I will be reviewing your branches. Could you also do some work to rustle up a second reviewer for your branches so we can move things along? | 14:31 |
| gary_poster | I am working on benji's now then will move to tveronezi's two | 14:31 |
| gary_poster | if anyone beats me to it that's fine by me ;-) | 14:31 |
| benji | tveronezi: I'll review yours if you review mine | 14:31 |
| gary_poster | :-) | 14:32 |
| benji | (Is this some sort of iterated prisoner's dilemma?) | 14:32 |
| gary_poster | heh | 14:33 |
| tveronezi | hahah. | 14:44 |
| tveronezi | :) ok. | 14:44 |
| tveronezi | benji ^ | 14:45 |
| benji | :) | 14:45 |
| Makyo | gary_poster, Sorry I left myself online last night! Sounds good for rejecting that mp. Let me make sure I'm good to push, I agree about the separate card for subordinate issues. | 14:52 |
| gary_poster | cool thanks Makyo (and np, I do that all the time for better or worse) | 14:53 |
| gary_poster | benji, approved your branch with comments & small requests | 15:11 |
| benji | gary_poster: thanks | 15:11 |
| gary_poster | welcome, thank you | 15:11 |
| benji | after my upgrade to 12.10, when I visit some sites in Firefox asks me if I want to install NAME-OF-SITE. I have said yes to some but I can't find anything in addons or anywhere else. | 15:15 |
| bac | benji: i've seen that and said no. wonder what it does | 15:20 |
| bac | hi mattuk1972, can i ask you about charm search results styling? | 15:21 |
| bac | namely the intent of list_one_div and list_two_div | 15:21 |
| benji | on a positive note, I figured out how to kill the persistent tooltips of death | 15:21 |
| frankban | mattuk1972: so, the border between elements in the charm panel is no longer one line dark gray and one line white. But it's one line dark gray and one line gray, right? With the exception of the first one after the "<- back" section | 15:22 |
| gary_poster | I think the browser thing has different integration depending on the site. The browser menu may get special options (which can be accessed with the HUD) in some cases; in other cases the site may install controls within the top-right sound menu, for instance, or other existing action menus | 15:22 |
| benji | interesting; it would be nice if I could figure out what each site's special features are | 15:25 |
| gary_poster | dunno, but yeah agreed | 15:31 |
| Makyo | gary_poster, tveronezi: Events are bound in attachSceneEvents, line 338 of environment.js. Since each event is bound in the same way with the same signature, we have to call them in the same way if we're to call them from other closures. I don't know if this is something that might change with refactoring, though. | 15:45 |
| Makyo | Not sure if that helps explain the pattern or not, bcsaller would probably be better at explaining :) | 15:47 |
| gary_poster | Makyo, but foo.bar() is identical to bar.call(foo) | 15:48 |
| gary_poster | so that at least can be simplified, right? | 15:48 |
| gary_poster | (from the latter to the former) | 15:49 |
| bcsaller | .call() supports chaining, foo.bar() doesn't by default | 15:49 |
| gary_poster | bcsaller, that's good to remember, but irrelevant unless we are chaining, yeah? | 15:51 |
| bcsaller | its useful in the sense that we say "if () {one line expression} because you'll often need it later and the cost is low | 15:52 |
| bcsaller | the braces I mean | 15:52 |
| * gary_poster not sold on this as a pattern we should therefore follow :-) (YAGNI in the small comes to mind) but maybe we ought to talk about it Friday | 15:53 | |
| Makyo | I have no stake in either. If it changes, it changes :) | 15:54 |
| gary_poster | :-) | 15:54 |
| mattuk1972 | frankban: the default divider is one grey one white - except on the ones I've marked - where there is suppose to be in inset shadow effect for example | 15:55 |
| tveronezi | Makyo, gary_poster, bcsaller: It just seems strange to see "this.myFunction.call(this)" but does not break the application. I am good having it if you agree. But don't make it as standard. It should be used only if really need it. | 15:55 |
| bcsaller | its not Method.call or Method.apply, its d3Selection.call() -> selection | 15:56 |
| bcsaller | I don't feel strongly either, but the d3 style is chaining | 15:57 |
| bcsaller | and remembering that something *doesn't* chain is more complex to me | 15:57 |
| gary_poster | bac bcsaller benji frankban jovan2 Makyo teknico tveronezi call in 2 | 15:58 |
| frankban | mattuk1972: ok, so, e.g. between description and interfaces, and between interfaces and changelog, it's gray-white. and for all: can you suggest a nice ruler to use with chromium? | 15:58 |
| bac | frankban: matt has left the building | 15:58 |
| bac | just as i was about to ask something | 15:58 |
| frankban | bac: yes... the ruler question is still valid | 15:59 |
| gary_poster | bcsaller, ack. maybe I don't know enough about context. Good friday conversation maybe | 15:59 |
| bac | frankban: yes, that would be handy | 15:59 |
| * frankban stops counting pixels and joins the hangout | 16:00 | |
| gary_poster | jovan2, starting | 16:01 |
| jovan2 | gary_poster on my way | 16:03 |
| hazmat | bac, jovan2 thanks, email sent | 16:29 |
| * bac -> lunch | 16:37 | |
| benji | Makyo: to kill the tooltips you: launch the CompizConfig Settings Manager, enable the Opacity, Brightness, and Saturation plugin, open that plugin, under "Windows specific settings" click the "New" button and enter "type=tooltip" (no quotes) and a value of 0. | 16:44 |
| Makyo | benji, does that kill all tooltips? | 16:45 |
| benji | Makyo: yep | 16:45 |
| benji | you can make the selector more specific to only kill some | 16:46 |
| Makyo | benji, Cool, thanks. Will try for a while, see if I miss anything | 16:46 |
| benji | or you can make the opacity very low instead of zero so they will still be there, but be less bothersome | 16:46 |
| * gary_poster lunches | 16:57 | |
| tveronezi | lunch... | 17:05 |
| bac | gary_poster: the cursor in the charm search box is inially red. mind if i make that gray? | 17:37 |
| gary_poster | bac +1 thanks | 17:37 |
| bac | s/inially/initially | 17:37 |
| gary_poster | it's the valid thing | 17:37 |
| gary_poster | you probably know that already | 17:37 |
| bac | gary_poster: it changes as soon as you type | 17:37 |
| gary_poster | bac, right it is a bootstrap valid thing | 17:37 |
| gary_poster | I | 17:37 |
| bac | righto | 17:37 |
| gary_poster | cool | 17:37 |
| bac | jovan2: can i ask you about charm search results styling? namely the intent of list_one_div and list_two_div | 17:40 |
| jovan2 | bac: I'll take a look... | 17:40 |
| jovan2 | bac: are you referring to visual design files? | 17:41 |
| bac | jovan2: yes | 17:41 |
| Makyo | gary_poster, do you have time for a quick call in a bit? I have a question re: this test. | 17:42 |
| jovan2 | bac: which zip or pdf file should I be looking at? | 17:42 |
| bac | jovan2: trying to find it on g+ | 17:43 |
| gary_poster | Makyo, happy to do it. IRC/email might be better in the short term, over next hour or two; after that call would be fine | 17:43 |
| Makyo | IRC's fine. | 17:43 |
| gary_poster | cool | 17:43 |
| bac | jovan2: https://docs.google.com/a/canonical.com/file/d/0B6l8lFdCRvtqNEZ4bElfVjF0Tjg/edit | 17:43 |
| bac | and the png files in charm_store_assets_1.zip | 17:44 |
| jovan2 | bac: it looks like a difference in background colour - white or grey | 17:45 |
| bac | jovan2: right, so there are three backgrounds then. one for selected and these other two | 17:45 |
| bac | is list_one_div really to be applied to the first in the list and the other non-hovered ones to be different colors? that is odd to me. | 17:46 |
| bcsaller | Has anyone else seen a problem with `make lint` running on recent branches (or maybe since the upgrade to Q)? | 17:46 |
| jovan2 | bac: yes it looks a bit odd to me too. Matt has left already, so will ask him to clarify tomorrow if that's ok? | 17:47 |
| bac | jovan2: sure. for now i'm going to proceed ignoring list_two_div | 17:48 |
| jovan2 | bac: ok | 17:48 |
| bac | frankban: is your charm panel description branch going to land today? i need your orange scrollbar | 18:02 |
| frankban | bac: yes, going to land asap | 18:03 |
| bac | yay | 18:03 |
| bac | hey gary, got a second for some pixel counting on stagin? | 18:39 |
| bac | g | 18:39 |
| frankban | bac: branch landed | 18:41 |
| bac | cool, thanks | 18:41 |
| tveronezi | benji: merge-files.js is entirely ours. :) | 18:48 |
| benji | tveronezi: ok, thanks; I'll finish the review here in a bit, then | 18:50 |
| === rog is now known as Guest70098 | ||
| bac | gary_poster, has anyone noted the extra width on the content div, making it hang off 1 px to the right. most visible when the charm panel is extended | 19:37 |
| gary_poster | I have but I think it is intermittent :-/ | 19:37 |
| bac | i see it with my branch but noted it is also on trunk. could not easily figure it out. | 19:37 |
| gary_poster | bac if you resize http://uistage.jujucharms.com:8080/ oevr and over again sometimes it will be just right and sometimes not. I was the last one to touch this code, and I made it a lot more reliable (IMO) but there's still this issue | 19:39 |
| bac | ok | 19:39 |
| bac | i'll make a card | 19:39 |
| gary_poster | ok thx | 19:39 |
| gary_poster | tveronezi, your Makefile has the same kind of problem that I talked with you about before, with the sprite generation: it runs the compression every time. This is one way to address it: http://pastebin.ubuntu.com/1340722/ | 19:50 |
| tveronezi | gary_poster: sweet... tkx. | 19:51 |
| gary_poster | welcome | 19:51 |
| tveronezi | gary_poster: we dont want to use $(NODE_TARGETS) instead of the node_modules/yui and node_modules/d3/d3.v2.min.js modules. Sure we will download unnecessary stuff, but it would be just once. | 19:55 |
| gary_poster | tveronezi, that's a question? | 19:55 |
| gary_poster | or a statement? :-) | 19:55 |
| tveronezi | gary_poster: yeap... question. :) | 19:55 |
| gary_poster | heh ok | 19:55 |
| gary_poster | I originally used $(NODE_TARGETS). I then wanted to be more specific about what this depends on. The most proper approach IMO would be to add app/assets/javascripts/d3.v2.min.js and *all* of app/assets/javascripts/yui/* (recursive) to $(NODE_TARGETS) (which is true--we're making those too) and then depending on the d3 and the yui in app/assets/javascripts | 19:58 |
| gary_poster | Listing yui was annoying | 19:59 |
| gary_poster | So but it would be good because that's what the script *actually* depends on | 19:59 |
| gary_poster | the combinejs script I mean | 19:59 |
| gary_poster | So then I compromised and said I only depended on nodejs and d23 | 19:59 |
| gary_poster | d3 | 19:59 |
| gary_poster | it will still build *all* of the node_modeules | 20:00 |
| gary_poster | there's no target for "only build these parts of node_modules" | 20:00 |
| gary_poster | so it is really just documentation | 20:00 |
| gary_poster | Specifying $(NODE_TARGETS) is *practically* exactly equivalent | 20:01 |
| gary_poster | tveronezi, ^^ | 20:01 |
| tveronezi | ok. | 20:02 |
| gary_poster | In sum, what I did was a documentation compromise. What you propose would be acceptable, and practically equivalent, but not in my picture of how things are really supposed to work with a Makefile. Since I'm only compromising, do whichever you prefer,. | 20:02 |
| gary_poster | If I were *really* doing it right, I'd feel stronger about it :-) | 20:02 |
| tveronezi | gary_poster: for some reason, using the $(NODE_TARGETS) does not work. The combinejs always runs three times if I use it. :/ I think I will stick with your first solution. | 20:05 |
| gary_poster | tveronezi, that suggests to me that the files listed in NODE_TARGETS are not quite right | 20:06 |
| gary_poster | so you could also try to to tackle it from that perspective | 20:06 |
| gary_poster | but do what you wish | 20:06 |
| tveronezi | gary_poster: thanks for your solution for the make files. I was struggling to get it working when I started the branch. I forgot to ask your help before submitting the branch. It works like a charm. | 20:08 |
| gary_poster | cool tveronezi | 20:08 |
| tveronezi | gary_poster: do you mind if I create another card for the $(NODE_TARGETS) issue? I mean, another card to pinpoint what is not supposed to be there? | 20:10 |
| gary_poster | tveronezi, if you wish. to be perfectly honest, I'd plan on forgetting about it until it bit me later :-P Certainly you don't need to address it in this branch IMO | 20:11 |
| tveronezi | gary_poster: ok... tkx. | 20:12 |
| * bac being summoned to the fort. bbiab. | 20:32 | |
| gary_poster | tveronezi, I submitted a preliminary review for the file aggregation branch; I'll have more later | 20:50 |
| tveronezi | gary_poster: ok... tkx. | 20:50 |
| gary_poster | bac, if you are available to review https://codereview.appspot.com/6819104/ then maybe tveronezi and I can review https://codereview.appspot.com/6817101/ | 20:53 |
| Makyo | Woo, have FF almost as fast as Chrome now. | 21:36 |
| gary_poster | Wow, Makyo cool | 21:37 |
| bac | gary_poster: ok. | 21:39 |
| bac | tveronezi: 2nd review done. thanks. | 21:46 |
| tveronezi | bac: tkx. | 21:46 |
| bac | thanks for the review gary | 21:48 |
| gary_poster | welcome | 21:48 |
| bac | i set the spacing to 10px as stated. perhaps something else is making it more. | 21:48 |
| bac | as to the different backgrounds, I asked jovan about it and he was going to check with matt. it didn't occur to either of us that it was alternating a la green-line | 21:49 |
| bac | if that is their intent, i think they didn't get it quite right on the vis design. makes sense, though. | 21:50 |
| gary_poster | bac 10px: oh ok. I didn't see the 10px directive, but agreed that this is what you have | 21:51 |
| gary_poster | so nm | 21:51 |
| bac | i wish the inspector gave a better way to measure | 21:51 |
| gary_poster | Makyo, I gave your branch a +1. You still need another look though | 21:57 |
| Makyo | gary_poster, Why thank ya | 21:57 |
| gary_poster | :-) | 21:57 |
| gary_poster | jbye | 22:14 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!