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

feat: pass markdown for hint message on field #5584

Merged
merged 3 commits into from
Jul 7, 2021

Conversation

gdgkirkley
Copy link
Contributor

Summary
This adds the option to include HTML in the hint message on a field. Closes #5559

The issue creator requested support for Markdown, which could be added in future but would require parsing the markdown from the config and would be a larger change. This is a simpler addition that addresses the same issue.

Test plan
I didn't see any existing tests for these components, but would be more than happy to add if I just missed them! This passes the existing tests.

image

A picture of a cute animal (not mandatory but encouraged)
Cute pup

@gdgkirkley gdgkirkley requested a review from a team July 5, 2021 01:45
@erezrokah erezrokah added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Jul 5, 2021
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

Thanks @gdgkirkley, can you look into supporting markdown and not HTML directly?

dev-test/config.yml Outdated Show resolved Hide resolved
@gdgkirkley
Copy link
Contributor Author

gdgkirkley commented Jul 6, 2021

Hey @erezrokah - I've updated to use react-markdown and removed that change to the dev config. react-markdown seemed like a great option since it can limit the allowed elements to just the bold, italic and link tags requested in the issue.

I also made an update to the docs - hope that makes sense for this change!

@gdgkirkley gdgkirkley changed the title feat: pass HTML for hint message on field feat: pass markdown for hint message on field Jul 6, 2021
@@ -328,7 +329,23 @@ class EditorControl extends React.Component {
/>
{fieldHint && (
<ControlHint active={isSelected || this.state.styleActive} error={hasErrors}>
{fieldHint}
<ReactMarkdown
allowedElements={['a', 'strong', 'em']}
Copy link
Contributor

Choose a reason for hiding this comment

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

em should be used to support italic.

Also, with unwrapDisallowed=true we don't need the p element or the added styling.

a: ({ node, ...props }) => (
<a
{...props}
target="_blank"
Copy link
Contributor

Choose a reason for hiding this comment

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

Open links in a new tab

{...props}
target="_blank"
rel="noopener noreferrer"
style={{ color: 'inherit' }}
Copy link
Contributor

@erezrokah erezrokah Jul 7, 2021

Choose a reason for hiding this comment

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

Inherit the color for links

Copy link
Contributor

@erezrokah erezrokah 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 the follow up @gdgkirkley!

I've made some minor changes to fix italic, open links in a new tabs and also apply the active/error style (active - when the field is focused, error when for example the field is required and the user clicks save).

image

image

@timtorres
Copy link

I was testing out the new support for strikethroughs in hints and saw that the link in the hint, enabled by this feat, has link styling that's overriding the hint styling, mainly font-size and font-weight. This doesn't seem like desired behavior, can someone confirm?

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support markdown in widget hint texts
3 participants