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: simple_format with blank wrapper_tag option returns plain html tag. #49120

Merged
merged 1 commit into from Sep 5, 2023

Conversation

akhilgkrishnan
Copy link
Member

@akhilgkrishnan akhilgkrishnan commented Sep 2, 2023

Partially Fixes #44948

By default simple_format method returns the text wrapped with <p>. But if we explicitly specify the wrapper_tag: nil in the options, it returns the text wrapped with <></> tag.

simple_format("Hello World")
# <p>Hello World</p>

simple_format("Hello World", {},  {wrapper_tag: "div"})
# <div>Hello World</div>

simple_format("Hello World", {},  {wrapper_tag: nil})
# <>Hello World</>

Detail

In this PR, it updates the behavior of the simple_format helper, If the user passes nil or an empty string as a value for wrapper_tag it defaults to use the <p> tag for wrapping.

simple_format("Hello World", {},  {wrapper_tag: "div"})
# <div>Hello World</div>

simple_format("Hello World", {},  {wrapper_tag: nil})
# <p>Hello World</p>

Co-authored-by: @JunichiIto

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the actionview label Sep 2, 2023
@akhilgkrishnan akhilgkrishnan force-pushed the simple-format-blank-wrapper branch 2 times, most recently from 33592ca to 6049da4 Compare September 2, 2023 15:40
@akhilgkrishnan akhilgkrishnan force-pushed the simple-format-blank-wrapper branch 2 times, most recently from 6c1bbd2 to c85deb4 Compare September 4, 2023 09:09
@rails-bot rails-bot bot added the actiontext label Sep 4, 2023
@akhilgkrishnan akhilgkrishnan force-pushed the simple-format-blank-wrapper branch 2 times, most recently from 0567538 to 0c71740 Compare September 4, 2023 09:12
actiontext/CHANGELOG.md Outdated Show resolved Hide resolved
@akhilgkrishnan akhilgkrishnan force-pushed the simple-format-blank-wrapper branch 2 times, most recently from c0fa23e to 26fd1ab Compare September 4, 2023 12:51
actionview/CHANGELOG.md Outdated Show resolved Hide resolved
@byroot
Copy link
Member

byroot commented Sep 5, 2023

Thanks, please just fix the changelog and after that I'll merge.

By default `simple_format` method returns the text wrapped with `<p>`. But if we explicitly specify
the `wrapper_tag: nil` in the options, it returns the text wrapped with `<></>` tag.

Before:
```ruby
 simple_format("Hello World", {},  { wrapper_tag: nil })
 # <>Hello World</>
```

After:
```ruby
 simple_format("Hello World", {},  { wrapper_tag: nil })
 # <p>Hello World</p>
```

Co-authored-by: Junichi Ito <jit@sonicgarden.jp>
@byroot byroot merged commit 11257c8 into rails:main Sep 5, 2023
3 of 4 checks passed
@byroot
Copy link
Member

byroot commented Sep 5, 2023

Thank you!

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.

simple_format with blank wrapper_tag option returns nonsence HTML tag
3 participants