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

Enabled HTML formatting for flashMessages #1017

Merged
merged 3 commits into from
May 31, 2017
Merged

Conversation

dariobanfi
Copy link
Contributor

As part of #878 (comment), successfully merged through #928 you enabled html formatting in error/success messages.

However this only work only through languageDictionary strings and not when the message is called through: lock.show({flashMessage: {...})

This small change should fix that.

@dariobanfi
Copy link
Contributor Author

(Failing tests shouldn't be because of my change I think... I just cloned the latest master)

@luisrudge
Copy link
Contributor

I triggered a rebuild. In the meantime, can you add a test for this? Thanks for the PR!

@dariobanfi
Copy link
Contributor Author

@luisrudge I added some component tests

@luisrudge
Copy link
Contributor

Hi @dariobanfi the tests are failing because you changed the way you render errors.
We validated https://github.com/auth0/lock/blob/f82d025fd9015d7e381174657f38e850d6dd1e69/test/mfa_ro.ui.test.js#L85 that the error was shown using this helper function. You should make sure the tests reflect the change you made and they pass. Let me know if you need more help. Thanks again for the PR!

@dariobanfi
Copy link
Contributor Author

I updated the PR.
The problem with tests was caused by an inconsistency on how GlobalMessage message prop was passed, sometimes it was a React component (when using i18n strings) and sometimes a string (when using flashMessage).
I changed so that it's always passed as React component.

@luisrudge
Copy link
Contributor

Thanks for the PR @dariobanfi 🎉

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.

2 participants