Hi,
Looks good
+1 for integration

Thanks Ankit for reviewing.

Submitting for integration review.

Hi Rossie,
These branches contain a large set of changes for MDL-30816 as well.
Does this issue need MDL-30816 integrated first, or is this a mistake in the branches?

Halting integration review until that is clarified.

Cheers
Sam

Hi Rosie, can you please get back to me about the branches and additional commits.

Cheers
Sam

Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

Hi Sam,

Sorry, I didn't notice your comment earlier.

This needs MDL-30816 to be integrated first.

Thanks.

Ok great thanks Rosie, have linked them now and will check out MDL-30816. May have to wait until next release now however.

Re-submitting for integration review.

Reopening as the blocking issue MDL-30816 has been reopened.

Updating patch for MDL-30816 and resubmitting this for integration review.

This issue has been integrated this week thanks Rosie.
Before it was integrated I did make one change, the id you used if far to generic and is used elsewhere so likely to cause XHTML issues.
I've renamed the id from 'user-'.$user->id to 'attempt-user'.$user->id.

Also just noting I will open a new issue now to fix the XHTML probs in lesson as there were a few when viewing the results screen.

Cheers
Sam

Sorry Rossie,

I am failing it for now as I am not sure if passing a link in label is accessible.

FYI:
According to w3c "When a LABEL element receives focus, it passes the focus on to its associated control.".
So how does this link get interpreted by accessibility tools?

<label for="attempt-user3"><a href="http://rajesh.moodle.local/im/user/view.php?id=3&amp;course=10" class="username">Student Number 1</a></label>

It might be nice to add label to input element then to a link.

If you feel otherwise, then please feel to change status to pass.

Thanks Raj.

I fixed it as suggested by Greg, as he has much more experiences in this field.

Adding Greg as watcher and comment for this issue.

Hi guys,

There's a good chance we are going to need to make a decision about this soon.
As its been integrated either it is going to stay or its going to be reverted.

It is very good thinking Raj, this is potentially fixing one accessibility issue while introducing another.
When looking at this myself I had imagined/assumed that screen readers would read out the label offering it as a link and then shifting onto the input.
Rosie I think it is best you test to see what happens. You could do it by creating a test page with just an label containing a link and an input and seeing what your screen reader does.
If it reads the link out, great, I think this can stay.
If not or focus gets messed up then we need to re-consider this fix and whether it gets reverted, or whether we leave it in there and open another issue to find a better solution.

Cheers
Sam

Thanks Sam,

I was reading a bit more about this and I think this needs re-thinking. According to http://www.456bereastreet.com/archive/200711/use_the_label_element_to_make_your_html_forms_accessible/
When checkboxes and radio buttons have properly associated labels, the label text will also be clickable, thus making the target area much larger and easier to hit. This obviously has usability benefits for all users. So if label text is clickable, then adding a link within clickable text might create confusion.

Sorry, I am no an expert here, but seems we are making it complicated. Not sure what is wrong in adding label at https://github.com/rwijaya/moodle/commit/27b5983be4c94edc5d0ff367f0f2a96adab8045a#L0R217

Ok cool, thanks for having a look/think about it Rosie.

If you're OK with it I will revert this changes and we can have a better look in coming weeks.
Sound OK?

It is fine with me Sam.

Update patch

Hi Sam,

I updated the patch by setting the label right above the select input.

Has been reverted now, thanks Rosie.

Code looks good Rossie,

I tested this as well.
Not sure if you would like to consider Sam's comment http://tracker.moodle.org/browse/MDL-33575?focusedCommentId=167439&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-167439

Yes Raj, I wanted to test that with screen reader but my screen reader is not being cooperative right now.

I'm trying to reconfigure my screen reader and report my finding later.

Sorry Rossie,

I was mentioning about id
Sam's comment:
I've renamed the id from 'user-'.$user->id to 'attempt-user'.$user->id.

Thanks for the reminder Raj.

Updated the patch and sending for integration review.

Hi Rosie,
Can you please rebase these changes, I got conflicts on 21, and 22 branches (did try 23, and master).

Thanks
Sam

ping

Sure Sam,

Fixing it right now.

Re-basing patch.

Thanks Sam.

Thanks Rosie, this has been integrated now

Thanks Rossie,

Checkbox is labelled correctly.

*Notice*: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26

Congratulations

!

You've made into Moodle

+

I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world.

cheers!