fluid-work IRC Logs-2012-08-23

[09:11:43 CDT(-0500)] <michelled> alexn1: I'm looking at your pull request for 4744 - can you tell me a bit about it? why did you need to introduce a tooltipContainer?

[09:12:16 CDT(-0500)] <alexn1> michelled: let me take a look into it to refresh my memory

[09:13:04 CDT(-0500)] <michelled> alexn1: also, it looks to me that you haven't updated your pull request for 4743 based on my review from the other day - is that right?

[09:14:27 CDT(-0500)] <alexn1> michelled: no I did not have a chance to look into 4743 yet

[09:14:27 CDT(-0500)] <michelled> anastasiac: did you get a chance to see what was going on with the combination of cindyli's capturing the escape key and your pull request to activate mute?

[09:14:53 CDT(-0500)] <anastasiac> michelled, no, I haven't figured that one out yet. I can focus on that today

[09:15:04 CDT(-0500)] <michelled> thx

[09:18:24 CDT(-0500)] <alexn1> 4744 - basically toggleButton will create and attach a tooltip to itself. Which means that it will show a tooltip when it gets focus and will update tooltip content depending on its model's state. toolTipContainer allows to attach a tooltip to another container where ToggleButton is part of (e.g. volume container with a slider and a togglebutton). Whenever user keyboard focuses on that html container a tool tip will be displayed and its conte

[09:18:30 CDT(-0500)] <alexn1> does it make any sense ?

[09:21:44 CDT(-0500)] <michelled> alexn1: yes, I think it makes sense. by default, an integrator would not specify a tooltip container, is that right?

[09:23:02 CDT(-0500)] <alexn1> yes by default ToggleButton would apply a tooltip to itself (again if the tooltip options are provided) and only in the case when integrator wants to attach a tooltip to another html element then you have to supply a tooltipContainer option

[09:24:06 CDT(-0500)] <michelled> ok, in your pull request, you have a modification to the CSS. it looks a bit odd: bottom: 29px; /* 20px is right on FF */

[09:24:09 CDT(-0500)] <alexn1> michelled: if you think that there is a better way of doing this feel free to say. It just made sense to me when a ToggleButton cannot be keyboard focused and a part of another container

[09:24:32 CDT(-0500)] <michelled> alexn1: it seems like a fine strategy to me

[09:24:35 CDT(-0500)] <alexn1> michelled: let me take a look at those css properties

[09:30:13 CDT(-0500)] <alexn1> michelled: ok this is to put volume slider above of the mute/unmute button. If you apply a tooltip to the container instead of the button then the slider overlays on top of the toggleButton.

[09:30:39 CDT(-0500)] <alexn1> oh yes and the comments. They are look a bit odd to me but I was worried to remove them

[09:31:39 CDT(-0500)] <michelled> alexn1: please look into them and see if they mean anything anymore.

[09:32:25 CDT(-0500)] <alexn1> I looked at them but I'm not sure what they mean

[09:34:04 CDT(-0500)] <michelled> maybe try and find out who wrote them and ask. and if you can't, and there's nothing wrong with the placement of the button and slider on FF we can assume we can remove the comments

[09:34:19 CDT(-0500)] <alexn1> michelled: ok will do

[09:34:20 CDT(-0500)] <michelled> alexn1: also, can you change the casing for tooltip container to follow the convention?

[09:34:30 CDT(-0500)] <michelled> tooltip instead of toolTip

[09:34:44 CDT(-0500)] <alexn1> michelled: oh yes yes!

[09:34:57 CDT(-0500)] <michelled> thx

[09:35:02 CDT(-0500)] <alexn1> I'm looking into OER C issue. Should I do Fluid first ?

[09:35:18 CDT(-0500)] <michelled> it's up to you alexn1

[09:35:25 CDT(-0500)] <alexn1> ok

[09:35:34 CDT(-0500)] <michelled> I'll put comments on the pull request

[09:35:42 CDT(-0500)] <alexn1> michelled: thanks !

[09:38:47 CDT(-0500)] <michelled> alexn1: I was just looking at your pull request for 4756 and I'm unable to recreate the issue with the current demo branch

[09:38:57 CDT(-0500)] <michelled> alexn1: does the bug still exist? how do I hear it?

[09:39:05 CDT(-0500)] <alexn1> michelled: let me take a look

[09:40:22 CDT(-0500)] <alexn1> michelled: me and Cindy both could recreate it. cindyli was the one who submitted it. Basically from what I remember you need to open video then start increasing volume using your keyboard controls. Once you reach over 100 you should lose sound for the demo video

[09:40:41 CDT(-0500)] <michelled> alexn1: you can still recreate it?

[09:40:52 CDT(-0500)] <alexn1> michelled: let me try

[09:43:10 CDT(-0500)] <alexn1> michelled: well so far it seems that I cannot recreate it simply because I cannot change the volume with the keyboard anymore

[09:43:28 CDT(-0500)] <alexn1> before you would keyboard focus on the video and press key up or key down to control the sound

[09:43:38 CDT(-0500)] <alexn1> but It does not work for me

[09:45:01 CDT(-0500)] <alexn1> if you look into the code when a videoPlayer in a focus then by pressing up and down it should control sound +/- 10 of the current sound

[09:48:55 CDT(-0500)] <michelled> alexn1: wow, we've got issues let's meet in the collab room and bring Mhairi with us

[10:17:50 CDT(-0500)] <michelled> alexn: now I'm looking at your pull request for full screen

[10:18:15 CDT(-0500)] <michelled> question for you - do we ever do full screen correctly right now? meaning, do we ever show our controls on the full screen UI?

[10:21:01 CDT(-0500)] <anastasiac> michelled, here's that web page we spoke about: http://wiki.fluidproject.org/display/docs/Working+With+UI+Options+On+Your+Site

[10:21:14 CDT(-0500)] <michelled> thx anastasiac

[10:23:26 CDT(-0500)] <michelled> looks good to me anastasiac

[10:23:42 CDT(-0500)] <michelled> I'm trying to remember all the things we've been doing on sites we put UIO on

[10:23:50 CDT(-0500)] <anastasiac> yeah, me too

[10:46:30 CDT(-0500)] <alexn> michelled: it is a built in fullscreen mode for the html5 video implemented in the browsers which support html5. We never had time to implement our own fullscreen mode. I think Safari has its own video controls when you do a fullscreen mode but not FF.

[10:47:44 CDT(-0500)] <michelled> alexn: then I think that we should remove the toggle button functionality from the interface for full screen. Meaning, let's add in a toggle button when it actually toggles something. for now, let's just have a plain old button

[10:48:02 CDT(-0500)] <michelled> alexn: right now are adding partial non working functionality and then working around it

[10:48:07 CDT(-0500)] <michelled> too much complexity

[10:49:07 CDT(-0500)] <alexn> well it is not only about the button unfortunately. The thing is that when we click a full screen button it will change fullscreen variable to be true in the model. But when we exit a fullscreen mode by pressing ESC this model variable does not change back to false

[10:49:21 CDT(-0500)] <alexn> the code I've added changes the fullScreen in the model

[10:49:32 CDT(-0500)] <alexn> hence the button which listens to this flag updates accordingly

[10:50:45 CDT(-0500)] <michelled> alexn: what I'm saying is that we should remove all that half working functionality. what does it mean for the model to have a fullscreen mode if we don't do anything?

[10:52:17 CDT(-0500)] <alexn> why we did not ask this question for the demo then? and why did we add this jira then?

[10:54:27 CDT(-0500)] <alexn> do we have a videoPlayer feature roadmap somewhere for this iteration so that I would know what is still considered to be "demo" related and what features should be removed from the videoPlayer?

[10:55:14 CDT(-0500)] <alexn> michelled. anastasiac, cindyli: jiras 4777 and 4778 are created. I also added them to the Floe Iteration Plan

[10:55:16 CDT(-0500)] <michelled> alexn: I guess we didn't think of asking this question.

[10:56:10 CDT(-0500)] <michelled> alexn: I guess part of each of our responsibilities is to try to look at the bigger picture and continually improve upon the code base we are working on. Just fixing a bug isn't really good enough. Although sometimes we have to do that when we have crazy time pressure.

[10:56:23 CDT(-0500)] <alexn> michelled: it is a good question since it is the second place where it seems that I'm making a hack around. First one is for tooltips and now the fullscreen one although I'm following tests and the structure of the current demo branch

[10:56:51 CDT(-0500)] <michelled> alexn: right, but we know there's a reason the code still lives in demo instead of master

[10:57:00 CDT(-0500)] <michelled> it needs to be improved before I'm willing to move it

[10:57:07 CDT(-0500)] <alexn> agreed

[10:57:24 CDT(-0500)] <alexn> so we need some specs or requirements for the next iteration to move it out of the demo

[10:57:45 CDT(-0500)] <michelled> alexn: I can be convinced that it's worth keeping the non working full screen stuff around, but from what I've seen of it so far, and your pull request, I'm not seeing its value

[10:57:50 CDT(-0500)] <michelled> what value do you think it has?

[10:58:08 CDT(-0500)] <alexn> value of the demo presentation if we have one

[10:58:27 CDT(-0500)] <michelled> what value does the non working fullscreen model have for a demo?

[10:58:28 CDT(-0500)] <alexn> any time soon. Or soon enough for us not being able to implement a complete fullscreen functionality

[10:59:17 CDT(-0500)] <michelled> alexn: I'm not saying to remove the button from the interface, nor the working behaviour of what that button does. All I'm saying is that we should remove all the hacks upon hacks relating to using a toggle button that doesn't actually toggle

[10:59:27 CDT(-0500)] <michelled> and making model changes that mean nothing to us in the current code base

[11:01:04 CDT(-0500)] <alexn> which brings again to the question - what are the current specs and requirements for this videoPlayer iteration so that I would know what I could possibly remove or write down - TBD once the full functionality is in.

[11:01:24 CDT(-0500)] <michelled> alexn: what sorts of specs and requirements are you looking for?

[11:01:37 CDT(-0500)] <michelled> I guess you are well aware that we work in small sprints - continually improving what we are working on

[11:03:05 CDT(-0500)] <michelled> alexn1: are you referring to what we need to do to get out of demo?

[11:06:04 CDT(-0500)] <michelled> alexn ?

[11:06:23 CDT(-0500)] <alexn> on the skype call will be a bit slow here in the channel

[11:06:30 CDT(-0500)] <michelled> ok

[11:17:43 CDT(-0500)] <alexn> michelled: anyhow… we are not delivering videoPlayer soon and we have enough time to work on all the features. So I will be removing/leaving all functionality which was built just for demo (like a html5 browser fullscreen mode or like tooltip functionality and tests built around a tooltip bug).

[11:19:34 CDT(-0500)] <alexn> There are lots of questionable places in the videoPlayer which were OK for the demo time but I assume this is the time when we starting to rip things apart and making functionality as we wanted before making those demo code patches.

[11:48:30 CDT(-0500)] <michelled> alexn: I think I wasn't very clear in what I am requiring before we move out of the demo branch. Here is the list:

[11:48:44 CDT(-0500)] <michelled> 1. All functionality that was working in the demo is still working

[11:49:01 CDT(-0500)] <michelled> 2. Testing supports are in place so that we reduce regressions

[11:49:14 CDT(-0500)] <michelled> 3. Captionator is forked correctly

[11:49:30 CDT(-0500)] <michelled> Please let me know if this is still ambiguous

[15:06:49 CDT(-0500)] <alexn> michelled: I changed FLUID-4661 pull request according to our conversation. I tried to put more information into the pull request comments as I could. I also have https://github.com/anvk/OER-Commons/tree/OER-725 which is based on staging branch fixing https://www.assembla.com/spaces/iskme/tickets/725

[15:07:27 CDT(-0500)] <alexn> michelled: I was looking into this another bigger OER issue but it slowly seems more work than I thought - https://www.assembla.com/spaces/iskme/tickets/733

[15:07:36 CDT(-0500)] <alexn> will try to close it today

[15:07:48 CDT(-0500)] <michelled> alexn: did you get a chance to test FLUID-4661 in IE, to make sure it doesn't throw errors?

[15:08:40 CDT(-0500)] <alexn> I checked and it does not

[15:08:41 CDT(-0500)] <michelled> alexn: I just looked at the pull request - the comments are great - very informative

[15:08:47 CDT(-0500)] <alexn> as mentioned in the comments

[15:09:00 CDT(-0500)] <alexn> although I could not test in IE8 since there are other errors being thrown first

[15:09:24 CDT(-0500)] <alexn> I guess cindy has been working on making it work

[15:11:04 CDT(-0500)] <michelled> alexn: I wonder if we should remove the button altogether in IE. what do you think?

[15:11:27 CDT(-0500)] <michelled> I guess we could do a check for whether the fullscreen functionality exists on the browser when we are rendering the UI

[15:12:17 CDT(-0500)] <alexn> michelled: we totally could do this… but maybe we should disable it instead of removing completely? This way we will show that the functionality is there but not for the specific browser and also we will keep the layout intact.

[15:12:46 CDT(-0500)] <alexn> by disabled I mean greyed out

[15:13:04 CDT(-0500)] <michelled> I'm not sure - jvass what do you think?

[15:13:24 CDT(-0500)] <michelled> is it better to not show functionality that is not available, or to show it greyed out?

[15:13:29 CDT(-0500)] <Justin_o> Bosmon: hello.. i have to leave soon again but i'm wondering for my problem from yesterday if it could be that the component doesn't exist when the invoker is trying to fire

[15:14:25 CDT(-0500)] <Bosmon> Justin_o - that's a possible explanation

[15:14:27 CDT(-0500)] <Justin_o> Bosmon: yura thought i should upgrade to the latest infusion as there may be changes in there that will fix this, however when is tried to do that i had other unit tests failing.. the strangest is that there is now an interaction between two vary simple tests for a simple renderer component.

[15:14:29 CDT(-0500)] <Bosmon> Can you think of a way that might happen?

[15:14:45 CDT(-0500)] <Justin_o> i'm running the tests and starting on the afterRender event, but the two tests still conflict

[15:15:06 CDT(-0500)] <Bosmon> Justin_o - the new jqunit is a bit more aggressive about the way it schedules tests, I have noticed

[15:15:38 CDT(-0500)] <michelled> alexn: jvass thinks we should remove it, but let's make another JIRA for that

[15:15:45 CDT(-0500)] <michelled> alexn: any chance you could make the JIRA?

[15:15:48 CDT(-0500)] <alexn> michelled: I can take of that

[15:15:53 CDT(-0500)] <michelled> thanks!

[15:16:20 CDT(-0500)] <Bosmon> Justin_o - did you make sure to rename your "markup test area" from id main to qunit-fixture?

[15:16:31 CDT(-0500)] <Justin_o> Bosmon: the component is created by the renderer.. i put that issue on hold and ran into again in another part of the code.. where the renderer will cycle between a few components depending on state

[15:16:39 CDT(-0500)] <Bosmon> That is usually the main cause of failure in markup-oriented tests when upgrading

[15:16:53 CDT(-0500)] <Justin_o> Bosmon: that could be it.. i didn't rename anything

[15:18:44 CDT(-0500)] <Justin_o> Bosmon: thanks that does seem to have fixed the test issue

[15:18:51 CDT(-0500)] <alexn> michelled: just a tiny question. I know you mentioned that we want to change tooltip to be toolTip in the videoPlayer code to be consistent… but I just searched for tooltip in the whole project and it seems that MyInfusion.js and TestUtils.js have "tooltip" everywhere (all non-capital letters).

[15:19:21 CDT(-0500)] <michelled> alexn: I actually wanted toolTip to be changed to tooltip

[15:19:26 CDT(-0500)] <michelled> to be consistent everywhere

[15:19:40 CDT(-0500)] <michelled> sorry if I wrote that the wrong way earlier

[15:23:05 CDT(-0500)] <Justin_o> Bosmon: in another test i'm now getting this error "options.instantiator is undefined"

[15:23:32 CDT(-0500)] <Bosmon> Justin_o - are you trying to use the instantiator?

[15:23:34 CDT(-0500)] <Justin_o> in that component i take in an array of component names, and have the renderer go through and create them all

[15:23:40 CDT(-0500)] <Justin_o> Bosmon: nope

[15:24:33 CDT(-0500)] <Bosmon> It looks like the standard renderer component workflow puts the instantiator into the rendererOptions

[15:24:58 CDT(-0500)] <Justin_o> I think i might have an example of this pushed up already, but it doesn't have the updated infusion yet

[15:25:05 CDT(-0500)] <Bosmon> But that doesn't look like it could cause the error you are seeing

[15:25:13 CDT(-0500)] <Bosmon> What line does that error occur on?

[15:25:18 CDT(-0500)] <alexn> michelled: aha I got it ! it is the function name I messed up since that.changeToolTip implies "toolTip: if we follow the camel case rules.

[15:25:24 CDT(-0500)] <alexn> will change that

[15:25:54 CDT(-0500)] <Justin_o> Bosmon: here's my test code https://bitbucket.org/jobara/decapod-0.6-ui-iteration5/src/12657f219548/tests/exporter/js/exportFormatGroupTests.js

[15:26:05 CDT(-0500)] <Justin_o> it's the last test that's broken

[15:26:29 CDT(-0500)] <Bosmon> Does the failure occur in your code or in the framework?

[15:26:39 CDT(-0500)] <Justin_o> Bosmon: here's the components code https://bitbucket.org/jobara/decapod-0.6-ui-iteration5/src/12657f219548/components/exporter/js/exportFormatGroup.js

[15:26:42 CDT(-0500)] <Bosmon> Ah, I see

[15:26:48 CDT(-0500)] <Justin_o> Bosmon: i think it's the framework.. i'll look it up.. one second

[15:26:53 CDT(-0500)] <Bosmon> Justin_o - unfortunately the API has changed for fluid.renderer.getDecoratorComponent

[15:27:07 CDT(-0500)] <Justin_o> Bosmon: oh yes.. i saw that.. didn't remember i was using it here..

[15:27:17 CDT(-0500)] <Bosmon> I did chat about this with colinclark... I think he decided to pass it since we had never advertised it in the documentation

[15:27:23 CDT(-0500)] <Bosmon> You are using it on line 95

[15:27:27 CDT(-0500)] <Bosmon> Which is most likely causing the failure

[15:27:28 CDT(-0500)] <Justin_o> yep.. there it is

[15:27:49 CDT(-0500)] <Bosmon> You need to dredge up the instantiator yourself now

[15:27:58 CDT(-0500)]

[15:28:21 CDT(-0500)] <Bosmon> Yes, ensuring that you spell it correctly : P

[15:29:00 CDT(-0500)] <Justin_o> Bosmon: well.. i'm likely not going to be able to do that consistently

[15:29:12 CDT(-0500)] <Bosmon> You have a defaults statement inside your test, which should not happen

[15:29:32 CDT(-0500)] <Justin_o> Bosmon: should that be outside of the test itself?

[15:29:39 CDT(-0500)] <Bosmon> Yes

[15:29:39 CDT(-0500)] <Justin_o> and what is the difference

[15:29:46 CDT(-0500)] <Bosmon> defaults statements should never be issued from code

[15:29:54 CDT(-0500)] <Bosmon> Unless there is an enormously good reason for it : P

[15:30:13 CDT(-0500)] <Justin_o> Bosmon: okay.. so even though it is just for the sake of that test

[15:30:16 CDT(-0500)] <Bosmon> demands neither

[15:30:29 CDT(-0500)] <Bosmon> Well, there is no real concept of "for that test" : P

[15:30:35 CDT(-0500)] <Bosmon> Test code should be like any other code

[15:30:55 CDT(-0500)] <Bosmon> The point is that after the test executes, the defaults statement is registered FOR ALL TIME

[15:31:00 CDT(-0500)] <Bosmon> So there's no real point pretending that it isn't

[15:31:09 CDT(-0500)] <Justin_o> Bosmon: ah makes sense.. it would just be confusing otherwise

[15:32:04 CDT(-0500)] <Justin_o> Bosmon: okay… so i think i should be able to fix my unit tests for the sake of the upgrade now.. but i still have that other issue..

[15:32:23 CDT(-0500)] <Justin_o> if it is because the renderer has removed the component will the framework now handle these cases?

[15:32:39 CDT(-0500)] <Bosmon> Justin_o - why might the component be removed?

[15:34:07 CDT(-0500)] <Justin_o> Bosmon: not sure for the case i talked to you about yesterday.. since that should have been around the time it was created.. so i guess there is a chance that it doesn't yet exist.. although that is a bit strange too

[15:34:46 CDT(-0500)] <Justin_o> in another case i ran into, the renderer is used to create/remove ui components .. since they are needed/not in the various states

[15:35:12 CDT(-0500)] <Justin_o> so for example, at one point there is a button, which becomes a progress indicator, which becomes the download links

[15:35:26 CDT(-0500)] <Justin_o> there are different components for each

[15:44:33 CDT(-0500)] <Justin_o> Bosmon: sorry.. i have to run again.. i'll try to get everything working with the upgraded infusion and see if i still have these errors.. i'm also attempting to map out the interaction between all of my components.. although it's proving to be quite gigantic

[15:44:42 CDT(-0500)] <Justin_o> Bosmon: talk to you next week