trix: Use p tag instead of div for regular block content

Currently regular blocks are a <div> with newlines added as a <br> (unless you convert a line to another type and back). It would allow more flexible styling (and arguably be more semantic) to default to a <p> tag with enter adding a new tag, and shift+enter adding a <br>. This could be added as an opt-in for backwards compatibility.

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Reactions: 41
  • Comments: 34 (4 by maintainers)

Commits related to this issue

Most upvoted comments

I have to agree with @Truffula that semantically, Enter is a new Paragraph, while Shift+Enter is a Linebreak - and that is what br tag stands for. Copy pasting text from Basecamp to Word clearly shows the mess. I don’t think Images are allowed inline presently anyway, in Basecamp they certainly do not behave like that. So it should not be difficult to simply close the <p> tag and open <figure> whenever user inserts an image. This change would make Trix also more accessible for screenreaders (a blind person could navigate - jump through paragraphs, which is currently impossible).

As a teacher and using Basecamp for classroom, I need to be considerate of visually impaired students, and while presently I do not have any, I am part of the research team trying to bridge the gap in educational tools and accessibility of information for impaired students.

I don’t think <p> will ever be the default block element in Trix. It’s a popular request though and I would like to allow them via configuration more easily. Currently, you can set Trix.config.blockAttributes.default.tagName = "p" in your app, which works fine, but doesn’t create a new <p> when you press <kbd>return</kbd>. https://github.com/basecamp/trix/pull/187 addresses that as does this (now stale) branch I started.

One potential issue with using <p> is Trix’s image attachments, which are rendered in a <figure> element. <p> only permits phrasing content so placing a <figure> in one would be invalid.

For what it’s worth, every word processing program (by default, at least) I’ve tried has the same newline behavior as Trix. Press <kbd>return</kbd>, cursor moves to the next line.

Thanks for the quick response! Do images need to be allowed inline, or could they be added in their own block?

As far as word processors go, I think if you turn on hidden characters, when you press return you get a paragraph mark (equivalent of </p><p>) — if your formatting has space after/before paragraphs you’ll get a larger break than the normal line spacing — whereas if you press shift+return you get a newline (equivalent of <br />).

Is #187 likely to be merged soon?

In any case, keep up the great work, and thanks for an awesome library! 😃

Here’s a small potential change that would allow configuring the default block to use <p> more naturally:

diff --git a/src/trix/models/block.coffee b/src/trix/models/block.coffee
index f426bc3..8fb3f9d 100644
--- a/src/trix/models/block.coffee
+++ b/src/trix/models/block.coffee
@@ -96,7 +96,7 @@ class Trix.Block extends Trix.Object
     getBlockConfig(@getLastAttribute())?.terminal
 
   breaksOnReturn: ->
-    getBlockConfig(@getLastAttribute())?.breakOnReturn
+    getBlockConfig(@getLastAttribute() ? "default").breakOnReturn
 
   findLineBreakInDirectionFromPosition: (direction, position) ->
     string = @toString()
diff --git a/src/trix/models/line_break_insertion.coffee b/src/trix/models/line_break_insertion.coffee
index 27b0657..93e45eb 100644
--- a/src/trix/models/line_break_insertion.coffee
+++ b/src/trix/models/line_break_insertion.coffee
@@ -15,7 +15,7 @@ class Trix.LineBreakInsertion
     if @block.hasAttributes() and @block.isListItem() and not @block.isEmpty()
       @startLocation.offset isnt 0
     else
-      @breaksOnReturn and @nextCharacter isnt "\n"
+      @breaksOnReturn unless @shouldBreakFormattedBlock()
 
   shouldBreakFormattedBlock: ->
     @block.hasAttributes() and not @block.isListItem() and

And then in your app:

Trix.config.blockAttributes.default.tagName = "p"
Trix.config.blockAttributes.default.breakOnReturn = true

Let me know what you think.

I found a solution for style here… available on this fork https://github.com/lazaronixon/trix/tree/contentify It will generate a output like:

<h1>title<h1>
<div class="p">paragraph1</div>
<div class="p">paragraph1</div>

You can configure paragraph class with:

Trix.config.css.paragraph = "trix-p"

package.json

"trix": "lazaronixon/trix#contentify"

Is there any reason that the change wasn’t merged.

As I said before:

One potential issue with using <p> is Trix’s image attachments, which are rendered in a <figure> element. <p> only permits phrasing content so placing a <figure> in one would be invalid.

If we “just” switched from <div> to <p>, inserting an attachment would result in invalid HTML like the following:

<p>the <figure>…</figure> quick</p>
<p>brown fox</p>

To see just how invalid that HTML is, try loading it in a browser:

screen shot 2019-02-06 at 11 50 59 am

So, that’s the main reason support for <p> hasn’t moved forward. It’s not a simple change and it’s not a high priority for us.

They never cared about accessibility all that much. Unfortunately.

I would argue that this functionality is pretty essential for anyone concerned about accessibility.

I can ensure you that we do care about accessibility (see https://m.signalvnoise.com/the-next-big-jump-in-basecamp-accessibility/ for one example). Can you help me understand how much accessibility would be improved by using <p>s? For example, how significant is the difference between these two HTML snippets when using a screen reader or other assistive device?

<div>
paragraph one
<br>
<br>
paragraph two
</div>
<p>paragraph one</p>
<p>paragraph two</p>

/cc @bergatron

@joeldrapper, that’d be great! Especially adding a test or two.

@javan I’ve tested your suggested changes quite throughly and everything appears to work very well. I’m running it live in production with paragraph tags and I’ve had no problems so far.

If I write some tests and submit a PR, would you consider merging this in? None of the original tests break if you don’t configure Trix to use paragraphs.

@javan this seams to work perfectly and it doesn’t break any tests. Is there any reason why this can’t be merged as a configureable option? I’d happily work on fixing any issues, but your solution seams perfect. 👌

@javan since this will be a requirement for my particular implementation of trix, and I suspect may be beneficial for others, is there any guidance on how we could best go about this?

My initial approach wouldn’t have been all that successful. After more thought it comes down to how Trix.BlockView#createNodes renders out to a Trix.TextView that then collates nodes for pieces. For paragraphs to work, and to keep pieces as they are, an intermediary step to split pieces by \n\n to form paragraphs (retaining the pieces attributes) and \n for <br /> is roughly the new approach I am considering. However, I am unsure how well this will go down with the HTMLParser.

Something roughly flowing like: BlockView#createNodes -> ParagraphGroupView? -> TextView -> PieceView

But in this approach I would like to respect that the option to use paragraphs is optional. So thats potentially another added detail.

I’ll give it a go in a fork branch. One way of learning more about trix’s ins, outs and what-have-yous 😃