fluid-work IRC Logs-2012-09-05

[09:14:14 CDT(-0500)] <alexn> michelled: I made few changes in my pull request and left some comments. Could you take a look https://github.com/fluid-project/videoPlayer/pull/43/files

[09:21:09 CDT(-0500)] <michelled> thanks alexn!

[10:04:42 CDT(-0500)] <michelled> anastasiac: can you take a look at this ticket and point Andrey K to the information he needs? https://www.assembla.com/spaces/iskme/tickets/755?comment=159965963

[10:04:55 CDT(-0500)] <anastasiac> sure, michelled

[10:04:59 CDT(-0500)] <michelled> thx

[10:08:25 CDT(-0500)] <michelled> anastasiac: probably also a good time to point both Andreys and Sergey to the docs you've created on keeping UIO working on a site.

[10:08:36 CDT(-0500)] <anastasiac> right, michelled

[10:08:40 CDT(-0500)] <michelled> but I think you should do that separately - probably in an assembla message

[10:11:58 CDT(-0500)] <michelled> hey alexn - I feel like we are having a misunderstanding on your pull request

[10:12:05 CDT(-0500)] <michelled> do you know what I mean by 'chaining'?

[10:13:46 CDT(-0500)] <alexn> it is either having a set of functions in a loop or just setting one function after another through object.attr(something).attr(something).attr(something)...

[10:14:22 CDT(-0500)] <michelled> it's the second thing - setting things one after another.

[10:14:27 CDT(-0500)] <alexn> michelled: I also did another 2 OER tickets could you please take a look https://github.com/anvk/OER-Commons/tree/OER-714 and https://github.com/anvk/OER-Commons/tree/OER-720

[10:14:38 CDT(-0500)] <alexn> michelled: ok gotcha will make the change then

[10:15:33 CDT(-0500)] <alexn> https://github.com/anvk/OER-Commons/compare/ISKME:staging...anvk:OER-714

[10:15:50 CDT(-0500)] <michelled> sure alexn

[10:16:17 CDT(-0500)] <michelled> alexn, anastasiac: let's have a Floe planning meeting after stand up today

[10:16:37 CDT(-0500)] <anastasiac> ok, michelled, but remember that there's and R&D meeting at noon

[10:16:48 CDT(-0500)] <michelled> yeah, we'll be quick I think

[10:18:01 CDT(-0500)] <michelled> alexn: there are arguments both for and against method chaining, just like most decisions we make in code. We tend to favour not chaining for a couple of reason. 1. it often makes the code easier to read and 2. it makes the code easier to debug

[10:24:18 CDT(-0500)] <alexn> michelled: is it the only one required change or you are still looking into my pull request ?

[10:28:21 CDT(-0500)] <michelled> alexn: is it possible to put the default tooltip container - the toggle button - in the defaults block and let options merging handle it instead of doing the merge yourself on line 88?

[10:29:27 CDT(-0500)] <alexn> michelled: not sure

[10:29:59 CDT(-0500)] <alexn> the default container is that.locate("button")

[10:30:10 CDT(-0500)] <alexn> I'm not sure if this could be placed into the default block

[10:30:41 CDT(-0500)]

[10:56:13 CDT(-0500)] <colinclark> hey yura1, kasper and I have been chatting about a bug one of our demoers has encountered with the GPII

[10:56:45 CDT(-0500)] <yura1> colinclark: oh ya, with the new structure or the old one ?

[10:56:50 CDT(-0500)] <colinclark> new one

[10:56:55 CDT(-0500)] <yura1> interesting

[10:57:02 CDT(-0500)] <yura1> what are the details ?

[10:57:09 CDT(-0500)] <colinclark> kasper managed to trace it back to the fact that, by default, the flow manager's server is now running on port 8080 instead of 8081, and the USB listeners hard coded (eek!) the URL to the Flow Manager

[10:57:20 CDT(-0500)] <colinclark> Looks like a simple fix to me

[10:57:24 CDT(-0500)] <colinclark> but I wanted to confirm with you

[10:57:40 CDT(-0500)] <colinclark> Is this the configuration used by default? https://github.com/GPII/universal/blob/master/gpii/configs/fm.ps.sr.dr.mm.development.json

[10:57:59 CDT(-0500)] <colinclark> And I'm assuming that it's just a quick addition of a "port" option to the server's configuration?

[10:58:04 CDT(-0500)] <yura1> colinclark: yes i believe that;s the case I was testing it with curl instead of usb

[10:58:14 CDT(-0500)] <kasper_> yura1: tsk tsk tsk

[10:58:15 CDT(-0500)] <yura1> colinclark: yes

[10:58:16 CDT(-0500)] <kasper_>

[10:58:18 CDT(-0500)] <colinclark> cool

[10:58:23 CDT(-0500)] <colinclark> kasper_: Give 'er

[11:01:05 CDT(-0500)] <colinclark> kasper_: If the addition of that option makes it all work, I'd say go ahead and make a pull request

[11:01:16 CDT(-0500)] <colinclark> And we should file a bug about our USB listeners hard-coding URLs

[11:01:30 CDT(-0500)] <kasper_> colinclark, yura1: cool, I'll do both in a bit

[11:01:36 CDT(-0500)] <colinclark> awesome

[11:02:03 CDT(-0500)] <yura1> kasper_: have you seen my comments for you pull request ?

[11:02:28 CDT(-0500)] <yura1> kasper_: tsk tsk tsk

[11:02:47 CDT(-0500)] <kasper_> haha

[11:03:10 CDT(-0500)] <kasper_> only briefly, but will check and fix soon

[11:06:24 CDT(-0500)] <sgithens> colinclark: Just saw your email

[11:06:33 CDT(-0500)] <sgithens> I can do the same time on Friday, but not thursday

[11:07:59 CDT(-0500)] <kasper_> colinclark, yura1: adding "port": 8081, to the server config file didn't seem to do much

[11:11:05 CDT(-0500)] <Bosmon> We should probably move it off those ports anyway

[11:11:19 CDT(-0500)] <Bosmon> Since right now it conflicts with the port number used by node-inspector

[11:15:50 CDT(-0500)] <kasper_> Bosmon: yeah, I guess it might still make sense to use those ports in demo setups - as to avoid any issue with blocked ports at conferences, etc

[11:16:15 CDT(-0500)] <kasper_> Bosmon: though if we run it all locally it wouldn't matter anyway

[11:21:23 CDT(-0500)] <thealphanerd> colinclark: how was the trip?

[11:23:24 CDT(-0500)] <kasper_> yura1: so it seems there are some config files located under node_modules/flowmanager/configs/[development,production].json

[11:23:31 CDT(-0500)] <kasper_> and that these are the ones used by default

[11:23:52 CDT(-0500)] <kasper_> is there a reason it's not the config files used in the config dir that is used?

[11:24:02 CDT(-0500)] <kasper_> and that not all of the config files are there

[11:34:32 CDT(-0500)] <kasper_> colinclark, yura1: could either of you review when you have the time https://github.com/GPII/universal/pull/49

[11:35:07 CDT(-0500)] <kasper_> just added the "port" option to all config files to ensure that running with any config file will work out of the box (or the port part anyway )

[11:56:16 CDT(-0500)] <yura1> kasper_: ayt?

[11:56:40 CDT(-0500)] <yura> my question is why not switch usb listener to fire to 8080 ?

[11:56:42 CDT(-0500)] <yura> instead

[12:02:16 CDT(-0500)] <kasper_> yura: I believe the USB listener in linux will have the same problem, as well as the RFID listener both places

[12:15:48 CDT(-0500)] <colinclark> hey thealphanerd

[12:15:51 CDT(-0500)] <colinclark> vacation was great

[12:15:59 CDT(-0500)] <colinclark> are you settled into stanford now?

[12:16:02 CDT(-0500)] <thealphanerd> glad to hear it!

[13:23:57 CDT(-0500)] <colinclark> yura: Can you take a look at kasper's pull request this afternoon if you get a chance?

[13:24:13 CDT(-0500)] <yura> colinclark: sure

[13:24:21 CDT(-0500)] <colinclark> thanks, that's awesome

[13:54:00 CDT(-0500)] <michelled> fluid-everyone: anyone want to join the community meeting remotely?

[13:54:45 CDT(-0500)] <michelled> Jon and anastasiac are talking about the workshops they've recently attended

[13:55:05 CDT(-0500)] <jessm> michelled: jameswy and i have a call we're supposed to be on

[13:55:47 CDT(-0500)] <jameswy> jessm, michelled: I'll join in locally until the call's reinstated.

[13:59:30 CDT(-0500)] <yura> kasper: your pull request should be in

[15:23:26 CDT(-0500)] <alexn1> michelled: I removed this triple exclamation statement from the pull request https://github.com/fluid-project/videoPlayer/pull/45

[15:24:18 CDT(-0500)] <colinclark> !!!

[15:26:38 CDT(-0500)] <alexn1> colinclark: yes I do not know why I put it. was very confused

[15:26:43 CDT(-0500)] <colinclark> NOT NOT NOT

[15:26:56 CDT(-0500)] <alexn1> I know that !! is a safe conversion of anything into a boolean

[15:27:04 CDT(-0500)] <alexn1> and then I applied ! to get a negative of that

[15:27:18 CDT(-0500)] <alexn1> I think I was worried of undefined to false conversion

[15:27:26 CDT(-0500)] <michelled> thx alexn1 - I'm still not going to put that pull request in until we spend some more time thinking about how to do feature detection instead of looking for a browser name

[15:27:29 CDT(-0500)] <alexn1> well it happens. still funny I know

[15:28:05 CDT(-0500)] <colinclark> Not not not a big deal, alexn1

[15:30:22 CDT(-0500)] <alexn1> michelled: actually I was looking into it and I have an idea since I still check for browser flag existence in a fullscreen button functionality. Maybe I can do this check for hiding/showing the fullscreen button

[15:30:26 CDT(-0500)] <alexn1> colinclark:

[15:31:09 CDT(-0500)] <colinclark> Why not just check for the appropriate vendor prefixed functions?

[15:36:21 CDT(-0500)] <michelled> colinclark: I guess that's what I meant by feature detection - is the thing there? good use is : no don't offer it

[15:36:51 CDT(-0500)] <colinclark> I don't see any reason why we can't entirely feature detect full screen mode. Can you, alexn1?

[15:36:53 CDT(-0500)] <michelled> colinclark: alexn1 and I had an offline conversation about doing that but I can't remember the outcome. I believe that he ran into some issues

[15:37:04 CDT(-0500)] <colinclark> What were the nature of the issues?

[15:37:17 CDT(-0500)] <michelled> hopefully he either remembers what they were or he can try again and we can get that pull request in

[15:37:50 CDT(-0500)] <alexn1> *trying to remember

[15:38:20 CDT(-0500)] <alexn1> colinclark: I'm trying to find how I would check for the function existence in the document since those functions are bind through addEventListener

[15:40:33 CDT(-0500)] <alexn1> michelled colinclark: oh yeah I remember. Bascally michelled had a point that we do not have a full screen functionality which we want so we might just go ahead and remove fullscreen flag and the whole fullscreen functionality we currently have. I made a point that the fullscreen button is a toggleButton and that all the pieces already present in the code - model fullscreen flag change with the tooltip being changed and that fullscreen button

[15:40:54 CDT(-0500)] <alexn1> only 1 piece we were missing is the part which would set fullscreen flag from true to false

[15:41:27 CDT(-0500)] <alexn1> which usually happens when user quits a browser fullscreen mode. So we would just add this flag flip on the browser fullscreen mode change

[15:42:12 CDT(-0500)] <alexn1> and since the code is all there once we have a fullscreen funcionality we want - we would just replace the piece of code(function) which does native browser fullscreen with the function we will write

[15:42:21 CDT(-0500)] <alexn1> leaving everything as it is right now

[15:42:58 CDT(-0500)] <alexn1> I guess it is in a nutshell about our offline conversation. Did I miss anything michelled ?

[15:43:50 CDT(-0500)] <colinclark> I can't say I quite understand

[15:44:30 CDT(-0500)] <colinclark> The pull request I'm looking at seems to hide the button that toggles fullscreen mode

[15:44:39 CDT(-0500)] <colinclark> based on which browser we're running on

[15:44:46 CDT(-0500)] <alexn1> yes

[15:45:05 CDT(-0500)] <alexn1> we have 2 pull requests concerning fullscreen functionality

[15:45:07 CDT(-0500)] <colinclark> Surely the code that this button is bound to is going to call the browser's requestFullscreen function

[15:45:16 CDT(-0500)] <colinclark> And that function will be vendor prefixed

[15:45:21 CDT(-0500)] <colinclark> is that correct?

[15:47:55 CDT(-0500)] <alexn1> yes

[15:48:09 CDT(-0500)] <alexn1> colinclark: but to check if the function exists we need an access to the video element

[15:48:19 CDT(-0500)] <colinclark> right

[15:48:35 CDT(-0500)] <colinclark> new Video() should do the trick

[15:48:42 CDT(-0500)] <colinclark> shouldn't it?

[15:48:57 CDT(-0500)] <alexn1> colinclark: I will be honest with you - I do not know. I could try

[15:49:43 CDT(-0500)] <colinclark> Looks like they can't be instantiated directly, so you'll need to be a bit trickier

[15:49:50 CDT(-0500)] <alexn1> currently video element is an element in the dom. and we get an access to it through jQuery

[15:50:02 CDT(-0500)] <alexn1> so not sure how it would work through Video()

[15:50:05 CDT(-0500)] <alexn1> but I can try to take a look

[15:50:20 CDT(-0500)] <colinclark> I think you're not quite following

[15:50:24 CDT(-0500)] <alexn1> maybe

[15:50:30 CDT(-0500)] <colinclark> a feature detection happens outside of the context of a particular component

[15:50:39 CDT(-0500)] <alexn1> true

[15:50:47 CDT(-0500)] <alexn1> I mean true true true

[15:50:48 CDT(-0500)] <colinclark> You'd create a video element, run some tests on it, and determine whether or not the feature exists

[15:51:35 CDT(-0500)] <alexn1> colinclark: exactly. I'm not sure if this would work. I will give it a shot, thanks for an idea

[15:51:50 CDT(-0500)] <alexn1> hopefully it will work. crossing fingers

[15:52:23 CDT(-0500)] <colinclark> This code is related to it all, and is pretty odd: https://github.com/fluid-project/videoPlayer/blob/demo/js/VideoPlayer.js#L443-449

[15:52:34 CDT(-0500)] <colinclark> Worth thinking about how to refactor that, too

[15:55:45 CDT(-0500)] <colinclark> alexn1: This seems to work nicely: $("<video />")[0].mozRequestFullScreen;

[15:56:34 CDT(-0500)] <alexn1> colinclark: sounds great. what about other browsers ?

[15:56:54 CDT(-0500)] <colinclark> I guess that points to my concern about the other code I mentioned

[15:57:17 CDT(-0500)] <alexn1> just tried Chrome and it works too

[15:57:25 CDT(-0500)] <alexn1> $("<video />")[0].webkitRequestFullScreen;

[15:57:27 CDT(-0500)] <alexn1> sweet

[15:57:31 CDT(-0500)] <colinclark> Ok, next step

[15:57:47 CDT(-0500)] <alexn1> all ears

[15:57:51 CDT(-0500)] <colinclark> How would you factor that into a cross-browser test for full screen support?

[15:59:16 CDT(-0500)] <colinclark> And to make it more fun, in relation to that other code, how would you do it without calling any functions?

[16:00:53 CDT(-0500)] <alexn1> colinclark: I'm not quiet following with the second statement. but what about first one - fullscreen functionality is a part of staticEnvironment. In the pull request ^ I mimmic it to hide the button

[16:01:02 CDT(-0500)] <alexn1> and to show as well

[16:01:16 CDT(-0500)] <colinclark> I'm actually asking you to type out the code here, alexn1

[16:01:19 CDT(-0500)] <colinclark> it'll be fun

[16:01:41 CDT(-0500)] <alexn1> well my existing tests are here https://github.com/fluid-project/videoPlayer/pull/45/files

[16:02:11 CDT(-0500)] <colinclark> right, that part I get

[16:02:15 CDT(-0500)] <colinclark> So, let me clarify

[16:02:32 CDT(-0500)] <colinclark> 1. michelled doesn't think your approach in this pull request is sufficiently robust

[16:02:36 CDT(-0500)] <colinclark> 2. I agree with her

[16:02:46 CDT(-0500)] <colinclark> 3. The solution is to use feature detection, rather than browser detection

[16:03:06 CDT(-0500)] <colinclark> So, given the this little snippet of code I pasted to you

[16:03:29 CDT(-0500)] <colinclark> How would you write those lines of code in your pull request?

[16:03:56 CDT(-0500)] <colinclark> or rewrite, rather

[16:04:23 CDT(-0500)] <colinclark> I mean this line of code from your pull request:

[16:04:24 CDT(-0500)] <colinclark> + var hasFullScreenMode = !($.browser.msie || $.browser.opera);

[16:04:33 CDT(-0500)] <alexn1> from the top of my head - most likely what I do. Check if there is the way to find the browser prefix depending on the current browser and if there is none then do a hash with the relation.

[16:04:41 CDT(-0500)] <alexn1> then

[16:05:27 CDT(-0500)] <colinclark> Best as I know, there's no way to determine a vendor prefix. So your first idea won't work.

[16:05:28 CDT(-0500)] <alexn1> check for existence of ($("<video />")[0][prefix + "RequestFullScreen"])

[16:05:38 CDT(-0500)] <colinclark> The second idea is curious

[16:05:42 CDT(-0500)] <colinclark> why a hash?

[16:05:45 CDT(-0500)] <colinclark> a hash of what?

[16:06:34 CDT(-0500)] <alexn1> to have a match between the browser and its prefix so that I could grab the proper value based on the browser type. there is a way to do in IF statement but this is not scalable since hash is only one data/model place if anything changes

[16:06:51 CDT(-0500)] <colinclark> okay

[16:06:59 CDT(-0500)] <colinclark> So it sounds like we have to have a look back at some of the basics

[16:07:07 CDT(-0500)] <colinclark> This actually pertains to your not not not issue

[16:07:16 CDT(-0500)] <colinclark> Pretend you're a JavaScript runtime

[16:07:19 CDT(-0500)] <colinclark> don't cheat with Firebug

[16:07:23 CDT(-0500)] <colinclark> What does this statement return?

[16:07:26 CDT(-0500)] <colinclark> !function () {};

[16:08:23 CDT(-0500)] <alexn1> never ran it. this is a really neat picky question. need to test it. can say that !(function () {}) should return false

[16:08:32 CDT(-0500)] <colinclark> YES!

[16:08:34 CDT(-0500)] <colinclark> correct

[16:08:51 CDT(-0500)] <colinclark> so we know that non-boolean things can be used in boolean expressions in JavaScript

[16:08:59 CDT(-0500)] <colinclark> Meaning, you can test things like functions, etc.

[16:09:07 CDT(-0500)] <alexn1> yup

[16:09:14 CDT(-0500)] <colinclark> Okay

[16:09:28 CDT(-0500)] <alexn1> using this in https://github.com/fluid-project/videoPlayer/blob/demo/js/VideoPlayer.js#L443-449

[16:09:54 CDT(-0500)] <colinclark> So, what happens if you run this code in Chrome, unmodified?

[16:09:55 CDT(-0500)] <colinclark> $("<video />")[0].mozRequestFullScreen;

[16:10:18 CDT(-0500)] <alexn1> it would run undefined;

[16:10:21 CDT(-0500)] <alexn1> undefined;

[16:10:24 CDT(-0500)] <colinclark> cool, yes

[16:10:27 CDT(-0500)] <alexn1> which in the end would not do anything

[16:10:33 CDT(-0500)] <colinclark> What would it do in IE and Opera?

[16:10:39 CDT(-0500)] <alexn1> same thing

[16:10:44 CDT(-0500)] <colinclark> ok

[16:11:07 CDT(-0500)] <colinclark> So, how about this

[16:11:26 CDT(-0500)] <alexn1> colinclark: our most interesting conversations always are around 5.

[16:11:27 CDT(-0500)] <colinclark> var requestFullScreen = <fill this in for any browser>;

[16:11:42 CDT(-0500)] <alexn1> not sure with the last statement

[16:11:47 CDT(-0500)] <alexn1> I mean not sure what is it

[16:11:55 CDT(-0500)] <alexn1> could you give more words on what you are doing

[16:11:55 CDT(-0500)] <colinclark> I want you to write that statement

[16:12:27 CDT(-0500)] <colinclark> I'm asking if it is possible to write a line of code that gives me the "requestFullScreen" function, regardless of what browser I'm on

[16:12:45 CDT(-0500)] <alexn1> it is a combination of 2 lines. Yours and mine ^ var requestFullScreen = ($("<video />")[0][prefix + "RequestFullScreen"])

[16:12:58 CDT(-0500)] <alexn1> I still do not know how to get the prefix though

[16:13:10 CDT(-0500)] <colinclark> well, try hard coding it

[16:13:16 CDT(-0500)] <colinclark> how would you write that statement>

[16:13:26 CDT(-0500)] <colinclark> given that you know the entire universe of vendor prefixes is bounded

[16:14:00 CDT(-0500)] <alexn1> requestFullScreen = ($("<video />")[0][fluid.getMePrefix(fluid.browserType) + "RequestFullScreen"])

[16:14:11 CDT(-0500)] <colinclark> Nice, but you called a function!

[16:14:16 CDT(-0500)] <colinclark> I'll give you the answer

[16:14:25 CDT(-0500)] <colinclark> var v = $("<video />")[0];

[16:14:57 CDT(-0500)] <alexn1> colinclark: are you still typing ?

[16:15:00 CDT(-0500)] <colinclark> var requestFullScreen = v.mozRequestFullScreen || v.webkitRequestFullScreen || v.oRequestFullScreen || v.msieRequestFullScreen;

[16:15:32 CDT(-0500)] <colinclark> No function calls, so it's very fast, and since it's a bounded universe of prefixes, probably acceptable in terms of the code duplication

[16:15:37 CDT(-0500)] <colinclark> Then you have yourself a function

[16:15:59 CDT(-0500)] <colinclark> you could call this function, say, in the fullscreen() method of video player

[16:16:10 CDT(-0500)] <colinclark> Or you could use it to evaluate whether or not the feature exists

[16:16:43 CDT(-0500)] <colinclark> such as return requestFullScreen ? fluid.typeTag("fluid.browser.hasFullScreenMode") : undefined;

[16:16:57 CDT(-0500)] <alexn1> looks great. Now I know what you mean hardcoded. I was assuming that I have to set the prefix to some hardcoded value

[16:17:14 CDT(-0500)] <alexn1> looks neat colinclark

[16:17:31 CDT(-0500)] <colinclark> So, can you summarize the key problem with the pull request?

[16:17:42 CDT(-0500)] <colinclark> Why we wouldn't want to include it in the code if possible?

[16:18:13 CDT(-0500)] <colinclark> Or, to put it differently?

[16:18:30 CDT(-0500)] <alexn1> oh I know lol. Michelle pointed out it long time ago. I was excluding Opera and IE and have to watch when they have functionality done and then modify my code. with the code you listed I do not need to wait for that

[16:18:37 CDT(-0500)] <colinclark> right

[16:18:58 CDT(-0500)] <colinclark> If a new release of Opera suddenly supports full screen mode, you don't have to go to the trouble of making a new pull request to fix it

[16:19:07 CDT(-0500)] <colinclark> not to mention spending days slogging away on a QA cycle

[16:20:02 CDT(-0500)] <alexn1> I just like to prefer hashes over hardcodings. even though this example is the best example of hardcoded values being so much more better

[16:20:12 CDT(-0500)] <colinclark> Well, that's the thing

[16:20:14 CDT(-0500)] <alexn1> do not think that browsers will change their prefixes any time soob

[16:20:16 CDT(-0500)] <alexn1> soon

[16:20:20 CDT(-0500)] <colinclark> I don't think a hash would solve your problem

[16:20:56 CDT(-0500)] <alexn1> it is just a nicer representation of the possible data

[16:21:00 CDT(-0500)] <colinclark> Well, it's not

[16:21:03 CDT(-0500)] <colinclark> an array, yes

[16:21:06 CDT(-0500)] <colinclark> a hash, not so much

[16:21:34 CDT(-0500)] <alexn1> well if it is an array you need to know what index belongs to which browser

[16:21:37 CDT(-0500)] <alexn1> isn't it ?

[16:21:55 CDT(-0500)] <colinclark> clearly this is a case where you just want to see "does this thing exist or not?"

[16:21:56 CDT(-0500)] <alexn1> I mean array of prefixes ["prefixone", "prefixtwo", ….]

[16:22:13 CDT(-0500)] <colinclark> you've got nothing to put on the other side of your hash, in this case

[16:22:48 CDT(-0500)] <alexn1> I mean you have hash of prefixes and then access it by hash.browsertype