Skip to content

Drag to resize paragraph#516

Closed
prabhjyotsingh wants to merge 2 commits intoapache:masterfrom
prabhjyotsingh:dragResizeParagraph
Closed

Drag to resize paragraph#516
prabhjyotsingh wants to merge 2 commits intoapache:masterfrom
prabhjyotsingh:dragResizeParagraph

Conversation

@prabhjyotsingh
Copy link
Copy Markdown
Contributor

dragresizeparagraph

@corneadoug
Copy link
Copy Markdown
Contributor

We already have one to resize the height of the paragraph result.
So there might be some conflicts

@prabhjyotsingh
Copy link
Copy Markdown
Contributor Author

Yes, I've noticed it, but it only shows up when there is a table/graph in view. Should I try merge it here, or this there?

@corneadoug
Copy link
Copy Markdown
Contributor

It's not too shocking to have both resize handles, however the resize graph height doesn't work anymore.
You might also want to rebase. (since we merged some graph width fix)
Maybe having one handle for both height and width could be better

@prabhjyotsingh
Copy link
Copy Markdown
Contributor Author

Have updated this PR, instead of having multiple resize handles. Merged it into one.

drag to resize paragraph

@Leemoonsoo
Copy link
Copy Markdown
Member

I have tested and it works really well. Thanks @prabhjyotsingh for really improvement.

One thing is, resize drag handle is shown in report mode. But i think it more sense to hide there. What do you think?

@corneadoug
Copy link
Copy Markdown
Contributor

Could you also un-commit the note.json?

@corneadoug
Copy link
Copy Markdown
Contributor

Found plenty of issues:

  1. Scrollbar when code line is too long on notebook refresh/load
    screen shot 2015-12-15 at 3 29 52 pm

  2. Resize to a size too small error
    screen shot 2015-12-15 at 3 30 37 pm
    screen shot 2015-12-15 at 3 30 53 pm

  3. opposite effect resize
    resize

@prabhjyotsingh
Copy link
Copy Markdown
Contributor Author

Thanks for the review and finding bugs;

  1. Scrollbar when code line is too long on notebook refresh/load - Fixed
  2. Resize to a size too small error - Fixed by having a minimum height, tried to use jQuery resizable minHeight but that cannot be used in out use case. Hence, fixed with following
var adjustHeight = elem.height() - extraHeight;
if (adjustHeight < 100) {
   scope.updateWidth({height: 100, width: Math.ceil(elem.width() / colStep)});
} else {
   scope.updateWidth({height: adjustHeight, width: Math.ceil(elem.width() / colStep)});
}
  1. opposite effect resize - With above two this also got fixed.

@prabhjyotsingh
Copy link
Copy Markdown
Contributor Author

Does this looks ok ?

@corneadoug
Copy link
Copy Markdown
Contributor

@prabhjyotsingh I will test it again tomorrow, sorry for the delay

@corneadoug
Copy link
Copy Markdown
Contributor

Tested, the bug are gone.
There is now a minimum size on the output.
However, would it be possible to apply the same limit to the dragging?
Right now, it looks like you can resize below that minimum height:
screen shot 2015-12-20 at 11 37 52 am

@prabhjyotsingh
Copy link
Copy Markdown
Contributor Author

Thanks @corneadoug for the feedback and a providing me with a lot of test cases.
Have merged this with the latest master, which seems to solve the bug for me.
Let me know if this works.

@Leemoonsoo
Copy link
Copy Markdown
Member

I've found weird behaviors.
I can make paragraph width larger than the notebook width and resize proxy is displayed incorrectly.
Please take a look following screenshot
paragraph_width

@prabhjyotsingh prabhjyotsingh force-pushed the dragResizeParagraph branch 3 times, most recently from b403ead to 8ce2abb Compare December 28, 2015 08:24
@Leemoonsoo
Copy link
Copy Markdown
Member

Tested and LGTM.
Thanks @prabhjyotsingh for really useful feature

@Leemoonsoo
Copy link
Copy Markdown
Member

Merge if there're no more discussions

@corneadoug
Copy link
Copy Markdown
Contributor

Re-tested, +1

@asfgit asfgit closed this in 4f18673 Dec 29, 2015
@prabhjyotsingh prabhjyotsingh deleted the dragResizeParagraph branch June 1, 2016 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants