[JRO] Add empty partials before/after post textarea #293
Conversation
jayroh
force-pushed
the
jro-textarea-before-after
branch
from
e58565f
to
ae6aeab
Compare
| <%= form.text_area :content, { rows: 5, required: true } %> | ||
| <%= render 'thredded/posts/after_content_field', form: form %> |
There was a problem hiding this comment.
I would say this should be just "after_content" (not "after_content_field"), since it's not a given that a field will be rendered there, and that this should be documented, but since this is an interim solution, LGTM anyway.
There was a problem hiding this comment.
I like your reasoning for the name - great point. And thanks for the
reminder on the documentation part. I got lost bouncing around branches
fixing rubocop stuff and forgot to take care of that part :)
On Wednesday, May 11, 2016, Gleb Mazovetskiy notifications@github.com
wrote:
In app/views/thredded/posts/_content_field.html.erb
#293 (comment):<%= form.text_area :content, { rows: 5, required: true } %>
- <%= render 'thredded/posts/after_content_field', form: form %>
I would say this should be just "after_content" (not
"after_content_field"), since it's not a given that a field will be
rendered there, and that this should be documented, but since this is an
interim solution, LGTM anyway.—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/thredded/thredded/pull/293/files/ae6aeabaca82e4d42d68e7d24799e35adab1ecf4#r62884576
There was a problem hiding this comment.
@glebm thinking about this a little more. What do you think about [before|after]_textarea for these partials instead? Because [before|after]_content could be misconstrued as being, literally, before and after a post's contents.
Plus there's only one textarea across this whole codebase, I think.
There was a problem hiding this comment.
Maybe posts/form/before_content?
There was a problem hiding this comment.
I think that's sufficient - surely.
|
@glebm renamed the two new partials and moved the content field partial to the same path so that things are reliably grouped. What do you think? |
|
👍 |
These two partials are merely for theming/overriding/extensibility for anyone who would like to add some functionality around the common posts textarea form element. Use cases for this include: Adding some sort of wysiwyg js boilerplate, adding buttons, links, or instructions for what markdown/bbcode is supported. NOTE: This is an initial step before implementing a more robust view customization API for thredded.
If we have a before and after form content partial, shouldn't they live alongside the actual form content partial? I would say that, yes, we should. This commit creates the common form subdirectory and moves these three partials in there, and renames the before and after. Higher level view partials pointing to these three have been updated to accommodate them.
jayroh
force-pushed
the
jro-textarea-before-after
branch
from
f369d80
to
45f5ec7
Compare
These two partials are merely for theming/overriding/extensibility for
anyone who would like to add some functionality around the common posts
textarea form element. Use cases for this include:
Adding some sort of wysiwyg js boilerplate, adding buttons, links, or
instructions for what markdown/bbcode is supported.
NOTE: This is an initial step before implementing a more robust view
customization API for thredded.