Skip to content
This repository has been archived by the owner on Jun 15, 2021. It is now read-only.

Do not insert BR in empty inline elements when parent node contains text #258

Merged
merged 3 commits into from Oct 2, 2014
Merged

Do not insert BR in empty inline elements when parent node contains text #258

merged 3 commits into from Oct 2, 2014

Conversation

AleLudovici
Copy link
Contributor

This solves the bug shown in the gif...
bold_space_bug

and the fix....
bold_space_fix

robinedman
robinedman reviewed Oct 1, 2014
View reviewed changes
src/plugins/core/formatters/html/ensure-selectable-containers.js
@@ -16,6 +16,12 @@ define([

// http://www.w3.org/TR/html-markup/syntax.html#syntax-elements
var html5VoidElements = ['AREA', 'BASE', 'BR', 'COL', 'COMMAND', 'EMBED', 'HR', 'IMG', 'INPUT', 'KEYGEN', 'LINK', 'META', 'PARAM', 'SOURCE', 'TRACK', 'WBR'];
var inlineElements = ['B', 'I'];
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be better if we'd use !isBlockElement in scribe-common. That way we should cover all types of elements, and also custom elements (which are inline by default).

@robinedman robinedman mentioned this pull request Oct 1, 2014
Closed
@AleLudovici
Copy link
Contributor Author

Scibe-common updated to use isCaretPositionNode. We also use !isBlockElement as suggested.

@hmgibson23
Copy link
Contributor

@robinedman. Mind taking a look?

On 2 October 2014 10:05, Alessandro Ludovici notifications@github.com
wrote:

Scibe-common updated to use isCaretPositionNode. We also use
!isBlockElement as suggested.

—
Reply to this email directly or view it on GitHub
#258 (comment).

Visit theguardian.com. On your mobile and tablet, download the Guardian
iPhone and Android apps theguardian.com/guardianapp and our tablet editions
theguardian.com/editions. Save up to 57% by subscribing to the Guardian
and Observer - choose the papers you want and get full digital access.
Visit subscribe.theguardian.com

This e-mail and all attachments are confidential and may also be
privileged. If you are not the named recipient, please notify the sender
and delete the e-mail and all attachments immediately. Do not disclose the
contents to another person. You may not use the information for any
purpose, or store, or copy, it in any way. Guardian News & Media Limited
is not liable for any computer viruses or other material transmitted with
or as part of this e-mail. You should employ virus checking software.

Guardian News & Media Limited is a member of Guardian Media Group plc. Registered
Office: PO Box 68164, Kings Place, 90 York Way, London, N1P 2AP. Registered
in England Number 908396

@robinedman
Copy link
Contributor

Sure thing. I'll pull and have a play around with this one! :)

@robinedman
Copy link
Contributor

Seems to be working fine (also for custom elements like <gu-note>, wohoo! ). And integration tests are passing. 👍

AleLudovici added a commit that referenced this pull request Oct 2, 2014
@AleLudovici
Merge pull request #258 from guardian/al-hg-fix-bold-spaces
9cfcc32 
Do not insert BR in empty inline elements when parent node contains text
@AleLudovici AleLudovici merged commit 9cfcc32 into master Oct 2, 2014
@robinedman robinedman mentioned this pull request Dec 12, 2014
Open
@jukecraft jukecraft deleted the al-hg-fix-bold-spaces branch September 4, 2018 11:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@AleLudovici @hmgibson23 @robinedman