Bug 52253 – remember diffstate for review tool

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ojan Vafai]

remember diffstate for review tool

[Ojan Vafai]

Created attachment 78598 [details]
Patch

[Ojan Vafai]

This builds on the patch in bug 52226.

[Adam Barth]

Comment on attachment 78598 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78598&action=review

> Websites/bugs.webkit.org/code-review.js:399
> +      localStorage.diffstate = difftype;

Can we use setItem and getItem?  Also, we should use a name like code-review-diffstate that scopes the state to the code review tool.

[Adam Barth]

Comment on attachment 78598 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78598&action=review

This patch is fine.  It could use slightly more polish though.

> Websites/bugs.webkit.org/code-review.js:744
> +    updateDiffState();

updateDiffState sounds like I can call this whenever I like, but this is really a load-time function.  Maybe loadDiffState?

> Websites/bugs.webkit.org/code-review.js:749
> +    if (localStorage.diffstate != 'sidebyside')
> +      return;

It seems like we should be more explicit here.  Maybe check that we've gotten one of the two expected values and then take action.

> Websites/bugs.webkit.org/code-review.js:753
> +    $('.FileDiff').each(function() {
> +      convertFileDiff('sidebyside', this);
> +    });

This chunk of code seems like it should be shared with the side-by-side link, right?

[Ojan Vafai]

Committed r75565: <http://trac.webkit.org/changeset/75565>