RESOLVED FIXED 53554
make draft comments focusable 
https://bugs.webkit.org/show_bug.cgi?id=53554
Summary make draft comments focusable
Ojan Vafai
Reported 2011-02-01 16:54:26 PST
make draft comments focusable
Attachments
Patch (3.26 KB, patch)
2011-02-01 16:55 PST, Ojan Vafai 		abarth: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Ojan Vafai
Comment 1 2011-02-01 16:55:57 PST
Created attachment 80856 [details] Patch
Adam Barth
Comment 2 2011-02-01 21:30:05 PST
Comment on attachment 80856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80856&action=review This is fine, but consider the comments below. BTW, I love j/k. :) > Websites/bugs.webkit.org/code-review.js:1280 > - if (node.hasClass(className)) { > + if (focus_type == FOCUS_TYPE.COMMENT && (node.hasClass('frozenComment') || node.hasClass('previousComment')) || > + focus_type == FOCUS_TYPE.DIFF_BLOCK && node.hasClass('DiffBlock')) { I'd just pass in a function that you call at this branch. That stops this code from having to know about all the different types.
Ojan Vafai
Comment 3 2011-02-01 22:00:34 PST
Committed r77363: <http://trac.webkit.org/changeset/77363>
Ojan Vafai
Comment 4 2011-02-01 22:03:17 PST
Comment on attachment 80856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80856&action=review >> Websites/bugs.webkit.org/code-review.js:1280 >> + focus_type == FOCUS_TYPE.DIFF_BLOCK && node.hasClass('DiffBlock')) { > > I'd just pass in a function that you call at this branch. That stops this code from having to know about all the different types. Done. That's much better.
Note You need to log in before you can comment on or make changes to this bug.