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

Replace dangeroulsySetInnerHtml in Abuse error with UnsafeRenderedMarkdown #27928

Merged
merged 2 commits into from Apr 17, 2019

Conversation

nkiruka
Copy link
Contributor

@nkiruka nkiruka commented Apr 8, 2019

Abuse error message is shown when users attempt to open a project that is marked as abusive.

Before

Screen Shot 2019-04-04 at 9 31 31 PM

After

Screen Shot 2019-04-05 at 3 20 19 PM

Copy link
Contributor

@Hamms Hamms left a comment

Choose a reason for hiding this comment

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

Looks like the textStyle style is no longer working with this change. Can that be fixed?

@nkiruka
Copy link
Contributor Author

nkiruka commented Apr 8, 2019

@Hamms Yes, that's right. Will adding a style prop to the UnsafeRenderedMarkdown component so styles can be passed into the component be an option?

@Hamms
Copy link
Contributor

Hamms commented Apr 8, 2019

Unfortunately, no; the paragraph elements these styles would be applied to are generated from the markdown rather than being developer-added React components to which we could apply the properties.

Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

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

In this case I think that's fine - we use this component in very particular circumstances and it looks okay.

@Hamms
Copy link
Contributor

Hamms commented Apr 8, 2019

In that case, I'd recommend removing the property entirely so as not to be misleading

@nkiruka nkiruka merged commit 3d69e2f into staging Apr 17, 2019
@nkiruka nkiruka deleted the abuse-error-mkdown-conv branch April 17, 2019 21:06
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.

None yet

3 participants