| Summary: | Give a resize handle for sidebyside diffs. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ojan Vafai <ojan> | ||||||
| Component: | New Bugs | Assignee: | Ojan Vafai <ojan> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abarth, eric, rniwa | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Ojan Vafai
2011-12-13 17:05:03 PST
Created attachment 119113 [details]
Patch
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? 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. Created attachment 119131 [details]
Patch
(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. Committed r102796: <http://trac.webkit.org/changeset/102796> |