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

Improve ActionText extensiblibility #40308

Merged
merged 1 commit into from Dec 30, 2020

Conversation

seanpdoyle
Copy link
Contributor

@seanpdoyle seanpdoyle commented Sep 30, 2020

Extensible layout

Expose how we render the HTML surrounding rich text content as an
extensible layouts/action_text/contents/_content.html.erb template to
encourage user-land customizations, while retaining private API control
over how the rich text itself is rendered by moving the
#render_action_text_content helper invocation to the
action_text/contents/_content.html.erb partial.

Extensible Attachable #to_attachable_partial_path

When an application declares a canonical partial for a record, there is
no way to override which partial is used when transformed to Rich Text.
For example, a default Person < ApplicationRecord instance returns
"people/person" from calls to #to_partial_path, resulting in the
app/views/people/_person.html.erb partial being rendered.

Prior to this change, when encountering an <action-text-attachment sgid="..."> element, ActionText retrieved the corresponding
Attachable instance (usually an ActiveRecord::Base instance) and
transformed it to rich text HTML by rendering the partial that
corresponds to its #to_partial_path.

This proposed change instead invokes
Attachable#to_attachable_partial_path. By default,
#to_attachable_partial_path is an alias for #to_partial_path.

Guides

Extend the guides/action_text_overview document to
describe how to customize these templates, and to better illustrate how
ActionText::Attachable instances are rendered into HTML.

@seanpdoyle
Copy link
Contributor Author

The prose changes introduced here were done in tandem with the implementation changes, which is to say that I'm very open to feedback on how to provide better ActionText guidance.

@seanpdoyle seanpdoyle changed the title Make ActionText layout extensible Document how ActionText layouts are extensible Sep 30, 2020
@seanpdoyle seanpdoyle force-pushed the action-text-rendering-guides branch 2 times, most recently from e100198 to 790c464 Compare December 8, 2020 01:10
@seanpdoyle seanpdoyle changed the title Document how ActionText layouts are extensible Improve ActionText extensiblibility Dec 8, 2020
@seanpdoyle seanpdoyle force-pushed the action-text-rendering-guides branch 2 times, most recently from df65a22 to 2797d0f Compare December 8, 2020 01:13
Copy link
Contributor

@abhaynikam abhaynikam left a comment

Choose a reason for hiding this comment

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

In documentation changes, I think ActionText -> Action Text because Rails components should have space in between the words. Refer: https://guides.rubyonrails.org/api_documentation_guidelines.html#wording

guides/source/action_text_overview.md Outdated Show resolved Hide resolved
guides/source/action_text_overview.md Outdated Show resolved Hide resolved
(known as blobs). On installation, Action Text will copy over a partial to
`app/views/active_storage/blobs/_blob.html.erb`, which you can specialize.

### Rendering attachments
Copy link
Contributor

Choose a reason for hiding this comment

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

This is awesome part. I always wanted to document this as currently their is no way anyone would know how to process and list attachments. Thank you @seanpdoyle

@@ -81,7 +81,7 @@ class Message < ApplicationRecord
end
```

Note that you don't need to add a `content` field to your `messages` table.
**Note:** you don't need to add a `content` field to your `messages` table.
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of field saying database column content sounds more readable to me. Please ignore if that doesn't sound okay to you. :)

guides/source/action_text_overview.md Outdated Show resolved Hide resolved
@georgeclaghorn
Copy link
Contributor

This is great. There a few other ActionText occurrences needing correction to Action Text but other than that, 👍.

@seanpdoyle seanpdoyle force-pushed the action-text-rendering-guides branch 2 times, most recently from dbc75dc to a358294 Compare December 8, 2020 23:05
@seanpdoyle
Copy link
Contributor Author

This is great. There a few other ActionText occurrences needing correction to Action Text but other than that, 👍.

Thanks for the review @georgeclaghorn. I've pushed up those Action Text changes. Would it also make sense to create a rails generate command to copy over the app/views/layouts/action_text/contents/_content.html.erb template?

Are there other documentation changes we'd need to make as well?

@seanpdoyle seanpdoyle force-pushed the action-text-rendering-guides branch 3 times, most recently from aa11a4e to 0d68477 Compare December 14, 2020 22:15
@@ -47,6 +47,9 @@ def create_actiontext_files

copy_file "#{GEM_ROOT}/app/views/active_storage/blobs/_blob.html.erb",
"app/views/active_storage/blobs/_blob.html.erb"

copy_file "#{GEM_ROOT}/app/views/layouts/action_text/contents/_content.html.erb",
"app/views/layouts/action_text/contents/_content.html.erb"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@georgeclaghorn I've pushed up a change to copy over the layout as part of rails generate action_text:install.

@seanpdoyle seanpdoyle force-pushed the action-text-rendering-guides branch 2 times, most recently from e72e24b to 95e1808 Compare December 29, 2020 20:43
Extensible layout
---

Expose how we render the HTML _surrounding_ rich text content as an
extensible `layouts/action_text/contents/_content.html.erb` template to
encourage user-land customizations, while retaining private API control
over how the rich text itself is rendered by moving the
`#render_action_text_content` helper invocation to the
`action_text/contents/_content.html.erb` partial.

Extensible Attachable `#to_attachable_partial_path`
---

When an application declares a canonical partial for a record, there is
no way to override which partial is used when transformed to Rich Text.
For example, a default `Person < ApplicationRecord` instance returns
`"people/person"` from calls to `#to_partial_path`, resulting in the
`app/views/people/_person.html.erb` partial being rendered.

Prior to this change, when encountering an `<action-text-attachment
sgid="...">` element, ActionText retrieved the corresponding
`Attachable` instance (usually an `ActiveRecord::Base` instance) and
transformed it to rich text HTML by rendering the partial that
corresponds to its `#to_partial_path`.

This proposed change instead invokes
`Attachable#to_attachable_partial_path`. By default,
`#to_attachable_partial_path` is an alias for `#to_partial_path`.

Guides
---

Extend the `guides/action_text_overview` document to
describe how to customize these templates, and to better illustrate how
ActionText::Attachable instances are rendered into HTML.
@rafaelfranca rafaelfranca merged commit f58b725 into rails:master Dec 30, 2020
@seanpdoyle seanpdoyle deleted the action-text-rendering-guides branch December 30, 2020 01:41
@sedubois
Copy link
Contributor

@seanpdoyle is something needed for these changes to become visible?

The source is correct: https://github.com/rails/rails/blob/master/guides/source/action_text_overview.md

But the website still shows the previous version (no section "Rendering Rich Text content"): https://edgeguides.rubyonrails.org/action_text_overview.html

Screenshot 2021-01-10 at 16 29 10

@seanpdoyle
Copy link
Contributor Author

@seanpdoyle is something needed for these changes to become visible?

I'm not sure, I don't think I've had a guides change merged before. Any ideas @rafaelfranca?

@WriterZephos
Copy link

WriterZephos commented Oct 15, 2021

Are these changes not in 6.1.4.1 or is something wrong with my gem installation because looking inside the gem, the changes are not there. I can't imagine they have been sitting for almost a year and never making it to a new version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants