Skip to content

Support HTML in help translations.#344

Closed
unikitty37 wants to merge 22 commits intobootstrap-ruby:masterfrom
unikitty37:master
Closed

Support HTML in help translations.#344
unikitty37 wants to merge 22 commits intobootstrap-ruby:masterfrom
unikitty37:master

Conversation

@unikitty37
Copy link
Copy Markdown

Currently, to use HTML in a help text that's stored in a translation, you would need to manually retrieve the translation and either wrap it with raw() or append .html_safe — the benefits of rails-bootstrap-forms being able to automatically retrieve the help text are lost. This PR allows translations to be marked as containing HTML.

In keeping with convention, the presence of HTML in a translation is
signalled by appending '_html' to the key name. It will then not be
escaped.

John Yeates added 3 commits September 2, 2017 14:29
In keeping with convention, the presence of HTML in a translation is
signalled by appending '_html' to the key name. It will then not be
escaped.
@edwardmp
Copy link
Copy Markdown

edwardmp commented Oct 23, 2017

Would be great if this can be merged in :)

@mattbrictson can you merge this in, as it solves #313

Copy link
Copy Markdown
Contributor

@lcreid lcreid left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! This is an excellent enhancement.

As you may have read in issue #361, we're a bit behind in merging pull requests. Our sincere apologies. We're now targeting Bootstrap 4 for all new development. Please fix the merge conflicts, and ensure that all tests still pass (Bootstrap 4 changes some of the output from the helpers).

Also, could you please consider adding a short note to the README.md file describing this behaviour, so users will know how to put HTML in translations?

Again, apologies for the delay in responding, and thanks for the contribution!

@unikitty37
Copy link
Copy Markdown
Author

I'll have a look at Bootstrap 4 compatibility now.

There's already a note added to README.md in a46a3ad — is that text OK?

@unikitty37
Copy link
Copy Markdown
Author

This is puzzling me — I had to add d31f240 to get the Travis builds to pass with Rails 4.0 and 4.1, but this seems to be causing test fails with Bootstrap 4.

Could somebody point me at what I should be doing here?

@lcreid
Copy link
Copy Markdown
Contributor

lcreid commented Jan 10, 2018

@unikitty37 Yes, the note in the README is good enough. I don't know how I missed that last night. Sorry!

@mattbrictson
Copy link
Copy Markdown
Contributor

@unikitty37 we are going to drop Rails < 5 from the Travis matrix soon, so don't worry about failures for 4.0 and 4.1.

@mattbrictson
Copy link
Copy Markdown
Contributor

@unikitty37 could you rebase your branch off of the latest master so we can verify tests pass against the latest code?

@lcreid was there anything else you wanted to see before approving this PR?

Copy link
Copy Markdown
Contributor

@lcreid lcreid left a comment

Choose a reason for hiding this comment

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

Good work!

(@mattbrictson is this the right way to say that they answered my previous review?)

r-sierra and others added 2 commits January 12, 2018 17:42
* Additional Form Group Attributes in select example

Show how to set additional Form Group Attributes like wrapper class

* Update README.md

Missing "}"
Minitest 5.11 is broken for environments, like Rails, that use custom
reporters. Pin to 5.10.3 in the meantime.
@GBH
Copy link
Copy Markdown
Contributor

GBH commented Jan 12, 2018

Syncing with master just brought in bunch of commits. Not possible to see what was done in this PR.

@unikitty37
Copy link
Copy Markdown
Author

Unfortunately I tried to rebase but it failed, and merge seems to have messed it up. What's the best way forward here? (and is there a guide to rebasing from the original repo? Github's docs use merges…)

@GBH
Copy link
Copy Markdown
Contributor

GBH commented Jan 12, 2018

git fetch upstream/master && git merge upsteam/master created this mess? My git knowledge is restricted to like 3 commands. I'd revert back to the commit before failed merge and try again with random things founds on stackoverflow.

@mattbrictson
Copy link
Copy Markdown
Contributor

No worries. Nothing is ever really lost in git. If the merge/rebase doesn't work, I can reconstruct a clean commit history and merge this in over the weekend.

mattbrictson added a commit that referenced this pull request Jan 13, 2018
@mattbrictson
Copy link
Copy Markdown
Contributor

Merged as 4622999. Thanks for the PR and congratulations on your first contribution to the project! 🎉

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants