Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix indent overflow #8

Closed
wants to merge 3 commits into from
Closed

Fix indent overflow #8

wants to merge 3 commits into from

Conversation

amitburst
Copy link

Fixes #4

The main issue here is that document.execCommand("indent") nests the selected line in blockquotes. Eventually, the summed margins of these nested blockquotes will cause the paragraph to overflow out of the editor div. I've fixed this by comparing the sum of blockquote margins to the width of the editor, so this is flexible if you later decide to change the default indent size.

I also moved the scripts to the bottom of index.html to ensure that DOM manipulation (needed for computing blockquote margin) functions correctly. It's good style anyway to put JavaScript at the end of the body, since it blocks rendering. :)

@@ -36,7 +56,9 @@
indent: {
iconClass: "icon-indent-right",
action: function(event) {
document.execCommand("indent");
if (rightIndentPixels() + blockQuoteMargin < parseInt($(".editor").css("width"))) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Callback action will be passed two parameters, event and popline itsself. Please use popline.target instead of hard code $(".editor").
Because the target selector is defined by user.

Thanks

@amitburst
Copy link
Author

My bad! Fixed this now.

// Left margin of a blockquote element.
var el = $("<blockquote></blockquote>");
var blockQuoteMargin = parseInt(el.hide().appendTo("body").css("marginLeft"));
el.remove();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amitburst It's better to put the margin calculate into $(document).ready. Wherever the scripts to be placed, it will always works.

@kenshin54
Copy link
Owner

Can you rebase it after fix the issue? Thx.

@kenshin54 kenshin54 closed this Sep 3, 2013
@amitburst amitburst deleted the fix-indent branch September 3, 2013 15:51
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.

Possible to indent text off the screen
2 participants