fluid-work IRC Logs-2011-02-14

[09:19:21 CST(-0600)] <heidi_> Justin_o i added two small uploader jiras that i'll work on fixing today: 4060 and 4061
[09:23:46 CST(-0600)] <Justin_o> heidi_: okay thanks
[10:23:55 CST(-0600)] <colinclark> michelled: How can I help with patch review and coding for today?
[10:24:23 CST(-0600)] <michelled> colinclark: I've got a cspace meeting after standup
[10:24:29 CST(-0600)] <michelled> after that I can switch to Infusion work
[10:24:43 CST(-0600)] <michelled> if you want, you can take a look at Harris' 3878 patch
[10:24:48 CST(-0600)] <michelled> branch I guess
[10:25:05 CST(-0600)] <colinclark> Okay. I started on that last week. I will continue
[10:25:15 CST(-0600)] <michelled> I got all heidi_'s patches in so harriswong's should be good to go now
[10:25:32 CST(-0600)] <colinclark> What's the next step with it? It was waiting on the CSS cleanup from heidi_, right? Which you committed this weekend?
[10:25:35 CST(-0600)] <colinclark> ah cool
[10:25:41 CST(-0600)] <colinclark> Meaning, it can be committed?
[10:25:50 CST(-0600)] <colinclark> or it needs to be reviewed specifically for CSS?
[10:25:50 CST(-0600)] <harriswong> michelled: revolving conflict at the moment
[10:26:06 CST(-0600)] <heidi_> michelled colinclark i checked fri's commits over and they look good. just two tiny bugs that ill work on today: 4060 and 4061
[10:26:33 CST(-0600)] <heidi_> colinclark the cleanup is committed
[10:26:39 CST(-0600)] <michelled> colinclark: it needs a full review but should be working correctly as far as CSS goes once harriswong is done with resolving the conflict
[10:26:53 CST(-0600)] <harriswong> colinclark: i think my it would be nice if heidi_ can take a quick look at my patch just in case it can be cleaned
[10:27:07 CST(-0600)] <heidi_> harriswong for sure let me know when it's ready
[10:44:25 CST(-0600)] <colinclark> harriswong: Is your fork ready for heidi_ to review?
[10:45:04 CST(-0600)] <harriswong> colinclark: nope, resolving conflicts
[10:46:12 CST(-0600)] <colinclark> k
[10:46:25 CST(-0600)] <colinclark> So I guess I'll hold off on reviewing it until you've got your conflicts resolved, too
[11:01:05 CST(-0600)] <harriswong> colinclark, heidi_: I have pushed the FLUID-3878 to here, https://github.com/harriswong/infusion/tree/FLUID-3878.
[11:01:29 CST(-0600)] <colinclark> thanks, harriswong!
[11:06:14 CST(-0600)] <Justin_o> fluid-everyone: for the code freeze.. in git we can set permissions based on group
[11:06:40 CST(-0600)] <Justin_o> which means we could say remove push access from the committers during code freeze and just give it to a subset, lets say, the release team..
[11:07:00 CST(-0600)] <colinclark> Are you thinking we'd make a Release Management group, give them write access during freeze, and turn it off for the rest of the community?
[11:07:03 CST(-0600)] <colinclark> Justin_o: ^
[11:07:06 CST(-0600)] <Justin_o> or we could just follow the same workflow we had with svn and trust that our committers will do the right thing
[11:07:14 CST(-0600)] <Justin_o> colinclark: yep
[11:07:24 CST(-0600)] <colinclark> I think the latter is probably safe to assume
[11:07:33 CST(-0600)] <colinclark> I'm not at all averse to formalizing it
[11:07:46 CST(-0600)] <colinclark> But I'm equally happy to leave it to the trust and responsibility of our community
[11:08:47 CST(-0600)] <Justin_o> colinclark: i'm fine either way, Option 1 is safer but has more overhead to manager, Option 2 has been working for years now..
[11:08:49 CST(-0600)] <Justin_o>
[11:09:22 CST(-0600)] <colinclark> Let's stick with option #2, then.
[11:09:32 CST(-0600)] <colinclark> We can make a note about the option of doing #1 in the release process documentation
[11:09:39 CST(-0600)] <colinclark> anastasiac: What do you think?
[11:09:52 CST(-0600)] <colinclark> btw, anastasiac, I just sent an email to you and the list offering to help out with the release process
[11:11:03 CST(-0600)] <anastasiac> colinclark, thanks. It seems to me that we should at least try to stick with the process we've been using so far (i.e. option 2). If we find out that it doesn't work so well with git, we can consider changing it for next time
[11:11:31 CST(-0600)] <colinclark> Seems reasonable to me
[11:11:56 CST(-0600)] <colinclark> I can't imagine any reason why the basic principle of "leave the code alone during freeze" won't apply equally well with Git and Github
[11:15:25 CST(-0600)] <heidi_> harriswong it looks like your patch contains changes already added to master (my upload patches michelled commited on fri), but not sure if that makes sense or not
[11:16:38 CST(-0600)] <Justin_o> colinclark: the only reason why i think it might be a little different is that people will easily be able to keep working and committing in their own space and may mistakenly push to the project repo
[11:16:54 CST(-0600)] <Justin_o> that being said, i think we don't have to worry about it now...
[11:18:10 CST(-0600)] <colinclark> Justin_o: That's actually a good point
[11:18:49 CST(-0600)] <colinclark> You're saying that the pull, change, commit, push workflow is such a natural part of Git that it's more likely for someone to mistakenly push from a clone to the project repo?
[11:20:20 CST(-0600)] <harriswong> heidi_: yep, i did a merge with the most recent trunk/
[11:20:59 CST(-0600)] <heidi_> harriswong how do i see which changes were just you, in uploader.css
[11:22:10 CST(-0600)] <harriswong> heidi_: mm.. my changes are gone. one sec.
[11:23:10 CST(-0600)] <harriswong> heidi_: In uploader/css/UPloader.css, starting on line 363, those should be all my changes on error hanlding.
[11:23:16 CST(-0600)] <harriswong> handling*
[11:23:36 CST(-0600)] <colinclark> heidi_: You can have a look at the changes for each of harriswong's commits
[11:23:50 CST(-0600)] <colinclark> Either from the command line by doing a log -p or right in Github
[11:24:02 CST(-0600)] <colinclark> So here's one of harriswong's key changes: https://github.com/harriswong/infusion/commit/7fff720bdb140048e099f6996b6023cb26d2cbb2
[11:24:35 CST(-0600)] <heidi_> colinclark thanks! somehow i ended up here instead: https://github.com/harriswong/infusion/commit/c598905c0c1fffe82ab69e0dc589455dc5dea09f#diff-3
[11:39:49 CST(-0600)] <heidi_> harriswong when i 'add more' are the old errors supposed to go away?
[11:40:24 CST(-0600)] <heidi_> also, how do i know the max # of files that can be in the queue?
[11:42:09 CST(-0600)] <heidi_> i kinda feel like the 'x' to close the error msg is in a weird spot... top right makes more sense to me
[11:43:06 CST(-0600)] <heidi_> also "which ones?" feels kinda weird. "Show list" / "Hide list" seems cleaner
[11:44:17 CST(-0600)] <colinclark> jameswy: ^
[11:45:02 CST(-0600)] <jameswy> colinclark, heidi_: what timestamp does this conversation start at?
[11:45:15 CST(-0600)] <heidi_> "Heads up!" is also kinda weird. i'd prefer just "Notices" or "Warnings" or "Errors"
[11:45:26 CST(-0600)] <heidi_> jameswy 12.39
[11:46:34 CST(-0600)] <harriswong> jameswy: heidi_ has a few comments on the error handler (http://issues.fluidproject.org/browse/FLUID-3878) ^
[11:46:34 CST(-0600)] <jameswy> heidi_: I'm guessing this is uploader?
[11:46:54 CST(-0600)] <heidi_> sorry jameswy yep i'm checking out harris's error stuff in uploader
[11:47:10 CST(-0600)] <jameswy> heidi_: Great questions. Answers:
[11:47:12 CST(-0600)] <harriswong> heidi_: no, the error stays till you click "Upload"
[11:47:26 CST(-0600)] <jameswy> Well, harriswong just answered one,
[11:47:35 CST(-0600)] <harriswong> lol
[11:49:19 CST(-0600)] <jameswy> heidi_: max number that can be added--that's a great observation. That was never in the original uploader design. The designers might've been thinking that it was the implementer's responsibility to add it to the context, but erin would know better.
[11:50:02 CST(-0600)] <jameswy> (erin, we're talking about uploader's lack of indication of the max number of files that can be added to the queue)
[11:50:25 CST(-0600)] <heidi_> harriswong i feel like "adding more" should be another round of error msgs, vs adding to the old errors
[11:50:32 CST(-0600)] <heidi_> it confused me
[11:50:48 CST(-0600)] <heidi_> esp if yr 'add more' doesn't cause any errors
[11:50:51 CST(-0600)] <jameswy> heidi_: re, "x" too close to the error message: I think it's in the most obvious space. The "x" are per error message, not for the entire panel.
[11:50:53 CST(-0600)] <heidi_> but you still get them
[11:51:24 CST(-0600)] <heidi_> jameswy yeah it just took me awhile to figure out that 'x' was a clickable button
[11:51:30 CST(-0600)] <heidi_> vs just a warning icon thing
[11:52:03 CST(-0600)] <jameswy> heidi_: "Which ones" vs "show/hide list" – the wording is up for grabs
[11:52:36 CST(-0600)] <erin> jameswy: just trying to refresh my memory... i think we specified rules re: error cases including max file number but you're right it doesn't seem to be incldued in the design
[11:52:48 CST(-0600)] <jameswy> heidi_: We can add more styling for users to realize that you can clear out the messages with the "x" button, but it's not a major offender. Most users aren't likely to clear it out at all.
[11:53:27 CST(-0600)] <jameswy> heidi_: "Heads up!" vs "Notices", "Warnings", "Errors" – again, wording's up for grabs.
[11:54:34 CST(-0600)] <heidi_> k thanks jameswy
[11:55:42 CST(-0600)] <harriswong> heidi_, jameswy: i think the queue stayed the same after "add more" is clicked is cause jameswy wanted to treat it as one "upload action", so all the errors are kept until "upload" is clicked, which indicate "this action has completed, and error can now go away"
[11:56:29 CST(-0600)] <heidi_> harriswong i think that makes sense if the errors are caused by actual uploading
[11:56:37 CST(-0600)] <heidi_> but most errors are caused by adding files to the queue, ya?
[11:56:51 CST(-0600)] <jameswy> heidi_: There are two kinds of errors--client and server side.
[11:56:58 CST(-0600)] <heidi_> ya
[11:57:10 CST(-0600)] <jameswy> heidi_: the ones under the status bar are ones we catch before actual uploading.
[11:57:24 CST(-0600)] <jameswy> and the ones inline with the file are server problems
[11:57:40 CST(-0600)] <jameswy> And yes, most errors are caused by adding files to the queue (hopefully)
[11:58:16 CST(-0600)] <jameswy> erin: Do you think indication of the max/current number of files in the queue should be added to the designs?
[11:58:46 CST(-0600)] <erin> jameswy: rather than giving an error when they select over max number, you mean?
[11:59:10 CST(-0600)] <jameswy> erin: Possibly both?
[11:59:28 CST(-0600)] <erin> jameswy: what is the limit?
[11:59:36 CST(-0600)] <heidi_> jameswy , yeah i was just confused that clicking 'add more' and adding more files still showed me the errors from the first 'browse files' action
[11:59:45 CST(-0600)] <jameswy> erin: It's defined by the implementer.
[12:00:26 CST(-0600)] <jameswy> heidi_: But the problem is that unless you've resolved those errors, the errors still remain--adding more files doesn't fix the sins of the past,
[12:01:28 CST(-0600)] <erin> jameswy: i don't like to clutter the interface, but indicating both the max number of files and max file size is not a bad idea
[12:01:29 CST(-0600)] <heidi_> jameswy how do you resolve the errors and make them go away?
[12:01:36 CST(-0600)] <erin> jameswy: in smaller grey font or something
[12:01:45 CST(-0600)] <heidi_> if i get the too many files error, delete a file, and add more, that error stays
[12:02:27 CST(-0600)] <jameswy> erin: Maybe we could do that, yes.
[12:02:59 CST(-0600)] <jameswy> heidi_: That's a good point. Let me mull over that.
[12:04:20 CST(-0600)] <heidi_> jameswy i guess i think of 'add more' as being your action to resolve things, a 'try again'. and should give me a clean list of probs. i guess, how does seeing the old errors matter/help?
[12:05:44 CST(-0600)] <jameswy> heidi_: I suppose I saw "add more" as a continuing on of the same session, of the same activity, as opposed to a "retry"
[12:05:47 CST(-0600)] <jameswy> Hm.
[12:06:46 CST(-0600)] <heidi_> but there's no action you can do to make them go away other than clicking the x ? it's hard to feel comfortable clicking 'upload' with a list of errors staring at you...
[12:07:45 CST(-0600)] <harriswong> heidi_: i think it may also serves as a 'log' or some sort?
[12:07:51 CST(-0600)] <heidi_> jameswy i also think the 'x' to the right feels better cos all the other file queue 'x's are there
[12:08:14 CST(-0600)] <harriswong> heidi_: like all the files that were skipped/not added to the queue
[12:09:10 CST(-0600)] <heidi_> harriswong just feels buggy to keep logs of an old action, once you've moved on
[12:09:14 CST(-0600)] <harriswong> heidi_, jameswy: But the 'x' image would align with the '!' above it, which makes user focus on it easily by looking at the 2nd row
[12:09:52 CST(-0600)] <heidi_> harriswong i think it does the opposite for me. i align it with the '!''s icon-ness and don't see it as a button
[12:11:25 CST(-0600)] <harriswong> heidi_: mm that make sense too, since the '!' is just a notification image, users might assume the whole column are just notification images as well..
[12:11:43 CST(-0600)] <heidi_> yeah it took me awhile to figure out i could click on it
[12:12:03 CST(-0600)] <harriswong> heidi_: haha i didn't have that problem cause i implemented it
[12:12:13 CST(-0600)] <heidi_> hehe
[12:12:30 CST(-0600)] <heidi_> okay im gonna look at your actual css now
[12:13:21 CST(-0600)] <harriswong> heidi_: Thanks!! (jameswy is working hard on re-thinking the uploader behind me)
[12:21:57 CST(-0600)] <jhung> fluid-everyone: We will be meeting in 10 minutesto discuss the new Infusion Builder design. Join us here - connect.yorku.ca/fluidwork
[12:36:00 CST(-0600)] <michelled> heidi_: one thing I found useful while reviewing branches was to do a diff between the last commit in the upstream and the last commit in the branch
[12:36:17 CST(-0600)] <michelled> that gives me the actual diff of what's going to be pushed
[12:36:28 CST(-0600)] <heidi_> michelled cool good tip
[12:36:36 CST(-0600)] <michelled> I used this command: git diff <commit> <commit>
[12:36:59 CST(-0600)] <heidi_> thanks
[13:30:29 CST(-0600)] <heidi_> harriswong so howcome there are two templates for client errors? one for file size, one for # files... couldn't use same template somehow?
[13:32:53 CST(-0600)] <harriswong> heidi_: Had a discussion with mlam and michelled on that, we concluded that it was easier to do 2 templates for now; since it's likely that we will use the renderer for uploader later.
[13:33:14 CST(-0600)] <heidi_> gotcha
[13:39:20 CST(-0600)] <heidi_> harriswong does .fl-uploader-queue .fl-uploader-file-state-error th get used? seems like there might be some extras styles here
[13:40:54 CST(-0600)] <Bosmon> Justin_o, jamon - sorry to miss our slot - I'm a bit poorly this morning
[13:41:12 CST(-0600)] <Justin_o> Bosmon: no problem.. will you be ready for 3pm
[13:41:25 CST(-0600)] <Bosmon> Oh, is it not yet?
[13:41:39 CST(-0600)] <Justin_o> colin stepped out for a minute, he'll be back for 3pm
[13:41:42 CST(-0600)] <Bosmon> ok
[13:41:47 CST(-0600)] <Justin_o> in the meantime I have some other things i can chat with you about
[13:41:49 CST(-0600)] <Justin_o> if you are free
[13:41:53 CST(-0600)] <Bosmon> That should be ok
[13:41:57 CST(-0600)] <harriswong> heidi_: it seems like it's repeated from line 135~155, it could be left over CSS from the merge. These might be used by the uploader file queue (not the error queue)
[13:42:00 CST(-0600)] <Bosmon> Perhaps 3.15 would be better
[13:42:08 CST(-0600)] <Justin_o> Bosmon: okay
[13:44:17 CST(-0600)] <heidi_> harriswong 133-155 can be removed looks like
[13:45:02 CST(-0600)] <heidi_> harriswong also 364-373
[13:45:38 CST(-0600)] <heidi_> harriswong should i just edit and push to yr branch somehow?
[13:48:36 CST(-0600)] <anastasiac> Jusin_o, is there a specific JIRA filter for 1.3.1-related issues?
[13:48:53 CST(-0600)] <heidi_> harriswong let's just be safe and remove 133-155, leave the rest. can you do that on yr end?
[13:51:01 CST(-0600)] <heidi_> harriswong and if you have a server side set up for uploader, can you check if .fl-uploader-file-state-error gets used at all?
[13:52:26 CST(-0600)] <anastasiac> Justin_o, is there a specific JIRA filter for 1.3.1-related issues?
[13:52:44 CST(-0600)] <harriswong> heidi_: sure, or i can give you permission to push. And no, i don't have a server side setup...
[13:53:09 CST(-0600)] <heidi_> harriswong ok just delete 133-155 duplicates for now, that should be fine
[13:53:33 CST(-0600)] <harriswong> heidi_: 133~135 is just the comments though?
[13:53:49 CST(-0600)] <heidi_> harriswong 155
[13:53:51 CST(-0600)] <harriswong> heidi_: oh my bad, i read 135 when you said 155.
[13:53:53 CST(-0600)] <heidi_> hehe
[13:54:19 CST(-0600)] <harriswong> heidi_: removed
[13:54:20 CST(-0600)] <heidi_> harriswong i think the rest is fine tho, except for the stuff i said earlier
[13:54:29 CST(-0600)] <heidi_> but css/html ok
[13:56:02 CST(-0600)] <heidi_> colinclark the uploader html doesn't validate because there are no "action" attributes in the form. is it worth putting action="" ?
[13:57:07 CST(-0600)] <colinclark> If it makes a validator pass, I guess it doesn't hurt
[13:57:20 CST(-0600)] <colinclark> but it's clearly a meaningless gesture for anything real
[13:57:26 CST(-0600)] <heidi_> yeah seems weird
[13:59:57 CST(-0600)] <colinclark> I don't mind either way, heidi_. If it makes it easy for using the validator, I say go fo rit
[14:00:11 CST(-0600)] <heidi_> harriswong do you want to add that in?
[14:07:33 CST(-0600)] <heidi_> harriswong what's the jira for the error issue
[14:07:38 CST(-0600)] <heidi_> ill add my prev comments to it
[14:13:57 CST(-0600)] <harriswong> heidi_: #3878
[14:14:04 CST(-0600)] <heidi_> right, duh
[14:14:47 CST(-0600)] <harriswong> heidi_, colinclark: James have updated (or updating) the wireframe for the error handler, i am going to update the code to reflect those changes, can you hold off for the review please. thanks.
[14:15:39 CST(-0600)] <colinclark> yep
[14:16:35 CST(-0600)] <EricDalquist> harriswong: I just posted a fixed version of the consistentGappedPageStrategy which passes the tests
[14:29:23 CST(-0600)] <harriswong> EricDalquist: thanks! I will download it and try it in an hour ish
[14:33:07 CST(-0600)] <jameswy> heidi_: I agree, "Add more" should clear out the warnings.
[14:33:24 CST(-0600)] <jameswy> heidi_: Added new mockups reflecting changes: http://issues.fluidproject.org/browse/FLUID-3878
[14:33:32 CST(-0600)] <heidi_> cool ill check it out
[14:33:50 CST(-0600)] <jameswy> The mockups don't show it, but all interactables should have a hover effect too.
[14:34:23 CST(-0600)] <jameswy> (namely, "clear error", "remove file", "upload", "browse", and "add more")
[14:34:35 CST(-0600)] <jameswy> (and "show/hide")
[14:35:13 CST(-0600)] <heidi_> jameswy i like it!
[14:37:17 CST(-0600)] <jameswy> Great,
[15:07:33 CST(-0600)] <harriswong> jameswy: is the 'x' intended to be grey? if so, can i get the image please. thanks.
[15:07:59 CST(-0600)] <jameswy> harriswong: If it's not intended to be grey, do you still need an image?
[15:09:55 CST(-0600)] <harriswong> jameswy: nope. i will reuse the red one.
[15:10:06 CST(-0600)] <jameswy> Aha.
[15:10:10 CST(-0600)] <jameswy> Yes, it should be grey.
[15:10:31 CST(-0600)] <harriswong> jameswy: ok! i will download it from the jira then
[15:11:11 CST(-0600)] <jameswy> I was just mentioning to heidi_ at 3:33 PM ^ that all the interactables should ideally have a hover effect.
[15:12:22 CST(-0600)] <michelled> golam: I can't remember - is your latest 3946 patch in a git branch?
[15:12:50 CST(-0600)] <golam> michelled: that's correct
[15:13:03 CST(-0600)] <michelled> can you comment on the JIRA with a link please?
[15:13:54 CST(-0600)] <golam> michelled: https://github.com/gchowdhury/infusion/tree/FLUID-3946
[15:14:19 CST(-0600)] <michelled> thx
[15:14:34 CST(-0600)] <golam> michelled: I will update the JIRA as well
[15:14:39 CST(-0600)] <michelled> perfect
[15:18:59 CST(-0600)] <jameswy> harriswong: It's in the JIRA now.
[15:19:26 CST(-0600)] <harriswong> jameswy: thanks