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

Allow rich text for Developer#bio #638

Merged
merged 16 commits into from Sep 16, 2022

Conversation

rayhanw
Copy link
Contributor

@rayhanw rayhanw commented Sep 15, 2022

Allows markdown for developer#bio. Closes #633

Pull request checklist

  • My code contains tests covering the code I modified
  • I linted and tested the project with bin/check
  • I added significant changes and product updates to the changelog

Notes:

  • Redcarpet markup can be found here.
  • I created a custom renderer object to handle links and images.
    If the user writes a markdown link/image, it will be saved into the DB but rendered as an empty string (check the custom renderer object above). I'm not sure if this is the best way, or whether actually removing it before the bio is saved would be better. Let me know!
  • I didn't write tests for the custom renderer object because I'm honestly not sure what to test of it and it should also be covered by the developers/rich_text concern tests.

Examples:

Text:

**Lorem ipsum dolor sit amet.**

Result:

<strong>Lorem ipsum dolor sit amet.</strong>

Text:

[link](https://example.com)
![image](https://example.com)

Result:

[link](https://example.com)
![image](https://example.com)

@joemasilotti
Copy link
Owner

joemasilotti commented Sep 15, 2022

Nice work so far!

I'd prefer to keep all of the user's inputted text for the bio instead of deleting anything. Can we instead render the markup directly, as in not an HTML <a> tag?

It looks like you are already disabling links and images via the HTML renderer. I think that's good enough and exactly what we want!

@rayhanw
Copy link
Contributor Author

rayhanw commented Sep 15, 2022

With the chosen options for the renderer and markdown, this is what it looks like now:
image

Might I also suggest adding in a text in the bio section in the form that says something along the lines of "Markdown is supported".

Should now be ready for review 👍

@rayhanw rayhanw marked this pull request as ready for review September 15, 2022 16:59
@joemasilotti
Copy link
Owner

Excellent! The preview you rendered looks perfect.

Let's add the "Markdown supported" text – great idea. Please use the following copy (it can be added below the EXAMPLE TOPICS list):

Markdown is supported but links and images are removed.

@rayhanw
Copy link
Contributor Author

rayhanw commented Sep 15, 2022

It is now just missing non-English translations 🙏

Copy link
Owner

@joemasilotti joemasilotti left a comment

Choose a reason for hiding this comment

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

Looks and works great! I only have two tiny requests.

CHANGELOG.md Outdated Show resolved Hide resolved
app/models/concerns/developers/rich_text.rb Outdated Show resolved Hide resolved
@joemasilotti
Copy link
Owner

Oh, I just noticed one more thing. Developer cards are rendering the plain markdown. Let's update that to remove the markup tags and render plain text. For example:

**hi** _bye_ should render as:

  • hi bye – on the developer profile page
  • hi bye – in a developer card

@rayhanw
Copy link
Contributor Author

rayhanw commented Sep 15, 2022

Looks like this now on the developer cards (with Redcarpet's stripdown renderer)
Screen Shot 2022-09-16 at 07 10 29

@joemasilotti
Copy link
Owner

This looks great! I made a few tiny tweaks and will merge after CI passes.

Thanks for your contribution, @rayhanw.

@joemasilotti joemasilotti merged commit 266faac into joemasilotti:main Sep 16, 2022
5 checks passed
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.

Rich text for developer bio
2 participants