Bug 74465 – Give a resize handle for sidebyside diffs.

• Component: New Bugs
• Status: RESOLVED FIXED

[Ojan Vafai]

Give a resize handle for sidebyside diffs.

[Ojan Vafai]

Created attachment 119113 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 119113 [details]
Patch

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

jQuery provides drag & drop support. Why are we not using it?

> Websites/bugs.webkit.org/code-review.js:67
> +  var minLeftSideWidth = 10;
> +  var maxLeftSideWidth = 90;

I'd rename them to something like minLeftSideRatio to signify the fact these are percent values.

> Websites/bugs.webkit.org/code-review.js:1728
> +  function generateFileDiffResizeStyleElement() {
> +    // FIXME: Once we support calc, we can replace this with something that uses the attribute value.

Really? We can't use CSSOM to generate style rules?

> Websites/bugs.webkit.org/code-review.js:1734
> +        'left: ' + i + '%' +

Can we make this a function? e.g.
cssValueWithPercent(name, value) { return value + ': ' + value + '%;' }

> Websites/bugs.webkit.org/code-review.js:1750
> +  $(document).bind('mousemove', function(e) {

I don't see why we don't want to spell out element.

> Websites/bugs.webkit.org/code-review.js:1751
> +    if (file_diff_being_resized) {

Can we do early exit instead?

[Ojan Vafai]

Comment on attachment 119113 [details]
Patch

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

>> Websites/bugs.webkit.org/code-review.js:1728
>> +    // FIXME: Once we support calc, we can replace this with something that uses the attribute value.
> 
> Really? We can't use CSSOM to generate style rules?

We could, but it would be considerably more verbose and less readable without any significant benefits.

>> Websites/bugs.webkit.org/code-review.js:1734
>> +        'left: ' + i + '%' +
> 
> Can we make this a function? e.g.
> cssValueWithPercent(name, value) { return value + ': ' + value + '%;' }

Could. It doesn't really seem worth the added complexity though. As it is, it's very clear what's going on.

[Ojan Vafai]

Created attachment 119131 [details]
Patch

[Ojan Vafai]

(In reply to comment #2)
> jQuery provides drag & drop support. Why are we not using it?

As best I can tell, It's not built for something like this. It's built for dragging absolutely positioned elements around.

[Ojan Vafai]

Committed r102796: <http://trac.webkit.org/changeset/102796>