Object Entry Code Review

[Svetoslav Nedkov]

Hi Justin, Sam,

I've attached a patch to 
http://issues.fluidproject.org/browse/ENGAGE-379 that adds the wrong 
code icon.

Sam, I've also added a aria-label attribute to the code entry fields, 
could you recommend a screen reader for Linux or Windows that recognizes 
ARIA attributes? Does the problem with redirection with iPhone persist?


Regards,

Svetoslav


Sambhavi Chandrashekar wrote:
> An issue I found while trying out object code entry on iPhone (both as 
> an app and through Safari without VoiceOver and with VoiceOver) is 
> that I am not transferred automatically to the artifact screen upon 
> entering the second digit. Whereas with Safari on Mac, upon entering 
> the second digit I get a message 'Opening artifact page' and the page 
> opens up.
>
> Sam
>
> On Fri, Feb 12, 2010 at 12:33 PM, Justin Obara <obara.justin at gmail.com 
> <mailto:obara.justin at gmail.com>> wrote:
>
>     Hi Sveto,
>
>     So I've committed your latest patch. I've assigned it to colin to
>     take another look at it though so he may have some more comments
>     coming.
>
>     I noticed that you worked in some of the changes that I had
>     mentioned, that's great.
>
>     One more thing to add though is for the unit  tests, it looks like
>     you could do some refactoring to get rid of some of the repeated
>     code. This is a mistake that I often make with unit tests.
>
>     Thanks
>     Justin
>
>     On 2010-02-11, at 8:19 PM, Justin Obara wrote:
>
>     > Hey Sveto,
>     >
>     > So I've taken  a look at your Object Entry Code patch and
>     general code. I have committed it into the repository with some
>     changes.
>     >
>     > Please see below for some thoughts. Hope it all makes sense.
>     >
>     > Thanks
>     > Justin
>     >
>     > Here are a few comments.
>     > ------------------------------------
>     >
>     > 1) You should always try to run jslint (http://www.jslint.com/)
>     before committing/patching. There was a bunch of changes that I
>     needed to make because of this, mostly due to tabs instead of
>     spaces (we use 4 spaces) and probably other things that were
>     caused by eclipse's auto-formatting.
>     >
>     > 2) We have a convention for our selectors. All css (styling
>     related selectors) begin with "fl-", all DOM selectors begin with
>     "flc-" if you need to both find and style an element you should
>     use two class names. Also here is the format we use
>     "flc-componentName-thing" and "fl-componentName-thing"
>     >
>     > Some changes I made.
>     > ---------------------------------
>     >
>     > You should probably take a look at the current code to see all
>     the changes, but I'll mention a couple here.
>     >
>     > 1) Internationalized the alt text for the delete button
>     >
>     > 2) Disabled the delete button while checking the validity of the
>     code
>     >
>     > 3) Changed the selector for the buttons, to have one for all the
>     digits and another one for the delete button
>     >
>     > 4) Removed some js files that were included but never used
>     >
>     > Things that still need to be done.
>     > --------------------------------------------
>     >
>     > (Note that these don't necessarily have to be done for 0.3b)
>     >
>     > 1) The atDigit logic seems a bit complex/confusing there may be
>     a better way to do this.
>     >
>     > 2) The service should be converted to use our new "spout"
>     method. You can see a demonstration of this in Colin's refactoring
>     of My Collection
>     >
>     > 3) The unit tests are broken. (note: i wasn't able to apply your
>     second patch because i had already made too many changes, but did
>     remove the test code manually. A new patch for the unit tests may
>     bring them back to life).
>     >
>     > 4) In the service you are returning whether or not an artifact
>     was found. Instead you should only return the necessary data (the
>     URL) on success and return an appropriate error message on failure
>     to find an artifact. The client side code should receive this
>     error and act upon it. That way for your ajax call, success would
>     be the transition to the correct artifact page, and the error
>     would be the invalid code message and resetting of the necessary
>     parts.
>
>     _______________________________________________________
>     fluid-work mailing list - fluid-work at fluidproject.org
>     <mailto:fluid-work at fluidproject.org>
>     To unsubscribe, change settings or access archives,
>     see http://fluidproject.org/mailman/listinfo/fluid-work
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://fluidproject.org/pipermail/fluid-work/attachments/20100215/a4c5ecdb/attachment.html>