FLUID-2936

[Colin Clark]

Alison,

On 15-Jul-09, at 10:38 AM, Alison Benjamin wrote:

> Hi,
>
> My first patch for FLUID-2936 made the progress unit test fail
> http://build.fluidproject.org/infusion/tests/component-tests/progress/html/Progress-test.html

Ack, sorry. This is something I should have caught during code review.  
Thanks for noticing.

> So I have written a second patch to address this. It affects two
> files. An assertion in ProgressTests.js was looking for the aria-live
> property, which our first patch removed. This second patch removes
> lines in ProgressTests.js that test for this. In Progress.js, the one
> change I made was in the initARIA function, where I added the line
> ariaElement.attr("aria-busy", "false"); .


I reviewed your code, verified that it does indeed correct the  
problem, and committed it.

One tip for making your patches easier to review: trivial changes to  
whitespace and line breaks can make it harder to see where substantial  
changes are made. For example, if the indenting or tabs/spaces change  
on a line, but the code itself  doesn't change, it can be harder to read

A minor detail for sure, but it's worth keeping in mind when you're  
creating your patches. If you're specifically making improvements to  
indenting, whitespace, or small style details, those are very  
worthwhile but are best done in a separate patch.

Nice work! This is a good improvement to the Progress component.

Colin

---
Colin Clark
Technical Lead, Fluid Project
Adaptive Technology Resource Centre, University of Toronto
http://fluidproject.org