Good work Ankit
There are couple of things you might want to consider before pushing it for integration review:
1. Spaces and tabs are used in code, it will be nice to replace them with spaces

git diff origin/master

look for red lines which indicate tabs.
2. You don't have to look for type of Y.one. Check can go like

if (firstelement === 'undefined' || !firstelement)

3. Focus is moved to right pane, but is selecting "refresh" text, even when files/folder are there in repository. I am not sure, but it might be nice to have focus on first file/folder if they are preset. I think Michael can provide some feedback on this.

Rajesh Taneja
added a comment - 12/Sep/11 1:06 PM - edited Good work Ankit
There are couple of things you might want to consider before pushing it for integration review:
1. Spaces and tabs are used in code, it will be nice to replace them with spaces
git diff origin/master
look for red lines which indicate tabs.
2. You don't have to look for type of Y.one. Check can go like
if (firstelement === 'undefined' || !firstelement)
3. Focus is moved to right pane, but is selecting "refresh" text, even when files/folder are there in repository. I am not sure, but it might be nice to have focus on first file/folder if they are preset. I think Michael can provide some feedback on this.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

Eloy Lafuente (stronk7)
added a comment - 16/Sep/11 8:03 AM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.
TIA and ciao

I'm sending this back this week I think the solution to this problem needs more thought, and a couple of minor things with the patch as well sorry Ankit.
In regards to the patch first up:

ln:431 The panel id has already been stored in a local variable and it would be much better to use that panel_id defined on ln:416 - In fact now that the panel node is being used more than once I think it would be better to store it within a local var rather than retrieve it through YUI twice.

The result of a Y.one call is either a node, or a null object. You only need to check if (something) and if you really want check it is not null. If we for the above create a local var for the panel we can check this in one place as presently the other use of that panel node assumes it is there and does not check.

However I think there are a couple of things we should discuss before implementing this patch.
Was the file picker ever designed with accessibility in mind? when playing with it I noticed that tabbing doesn't work within the filepicker for me (both before and after this patch) so even though we set the focus here it doesn't appear to be useable. I'm not entirely sure what is going wrong here I haven't looked into it perhaps it is something minor.
In regards to shifting focus it is normally seen as a negative to move focus in regards to the likes of screen readers as it throws the user into the middle of a structure and they have no clue where they actually are in the greater scheme of things. Perhaps this file picker popup is an exception as it is like opening a new window.

I think there are two things we need to do for this issue:

Bring this to Dongshengs attention (I've added him as a watcher). He was the original author of the filepicker and can probably shed some light on decisions made about accessibility.

Find an accessibility expert to have a really good look at the filepicker and its usability in whole.

Sam Hemelryk
added a comment - 19/Sep/11 8:57 AM Hi guys,
I'm sending this back this week I think the solution to this problem needs more thought, and a couple of minor things with the patch as well sorry Ankit.
In regards to the patch first up:
ln:431 The panel id has already been stored in a local variable and it would be much better to use that panel_id defined on ln:416 - In fact now that the panel node is being used more than once I think it would be better to store it within a local var rather than retrieve it through YUI twice.
The result of a Y.one call is either a node, or a null object. You only need to check if (something) and if you really want check it is not null. If we for the above create a local var for the panel we can check this in one place as presently the other use of that panel node assumes it is there and does not check.
However I think there are a couple of things we should discuss before implementing this patch.
Was the file picker ever designed with accessibility in mind? when playing with it I noticed that tabbing doesn't work within the filepicker for me (both before and after this patch) so even though we set the focus here it doesn't appear to be useable. I'm not entirely sure what is going wrong here I haven't looked into it perhaps it is something minor.
In regards to shifting focus it is normally seen as a negative to move focus in regards to the likes of screen readers as it throws the user into the middle of a structure and they have no clue where they actually are in the greater scheme of things. Perhaps this file picker popup is an exception as it is like opening a new window.
I think there are two things we need to do for this issue:
Bring this to Dongshengs attention (I've added him as a watcher). He was the original author of the filepicker and can probably shed some light on decisions made about accessibility.
Find an accessibility expert to have a really good look at the filepicker and its usability in whole.
Cheers
Sam

We have reached an point in this work where a decision needs to be made, and in order to make this decision, it would be good to get feedback from someone who uses a screen reader or knows how they behave and what is expected. Perhaps that is Mark, the person who originally launched this issue.

We can shift the focus from the repository to the right pane using the solution Ankit has created, however, we are not sure this is desirable, and for some repositories, it there is nothing that the focus can be shifted to.

I propose we do two things:

Leave the current solution out of integration until we get authoritative feedback on the issue, and

Create a second issue that address potential for things to be targeted in the right pane (even if we don't shift focus automatically, it is important that focus can be shifted manually, without a mouse-click).

Michael de Raadt
added a comment - 19/Sep/11 11:20 AM We have reached an point in this work where a decision needs to be made, and in order to make this decision, it would be good to get feedback from someone who uses a screen reader or knows how they behave and what is expected. Perhaps that is Mark, the person who originally launched this issue.
We can shift the focus from the repository to the right pane using the solution Ankit has created, however, we are not sure this is desirable, and for some repositories, it there is nothing that the focus can be shifted to.
I propose we do two things:
Leave the current solution out of integration until we get authoritative feedback on the issue, and
Create a second issue that address potential for things to be targeted in the right pane (even if we don't shift focus automatically, it is important that focus can be shifted manually, without a mouse-click).

Thanks to everyone for their work on this issue to date. I'm sorry I have not yet had time to install the patch and test the solution with a screenreader. I will make this a priority and post my feedback here.

Mark Sadecki
added a comment - 27/Sep/11 2:03 AM Thanks to everyone for their work on this issue to date. I'm sorry I have not yet had time to install the patch and test the solution with a screenreader. I will make this a priority and post my feedback here.

Michael de Raadt
added a comment - 17/Oct/11 1:17 PM Hi, Mark.
It would be good to get your feedback on this.
I suspect this will not solve the accessibility problem you raised, but we have raised the other related issue to work on in future.

Michael de Raadt
added a comment - 03/Nov/11 11:55 AM I'm brining this into the current sprint as we are still waiting for external feedback. We will look at this again at the end of the current sprint (next week).

Michael de Raadt
added a comment - 08/Nov/11 11:48 AM I'm closing this as deferred as I think we have explored what we can in this issue and launched a new issue to deal with an extension of the problem.