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.

Sign up for GitHub

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

Jump to bottom

[GH-19] Add markdown support to Todos #20

Merged
merged 6 commits into from Jan 13, 2020
Merged

[GH-19] Add markdown support to Todos #20

merged 6 commits into from Jan 13, 2020

Conversation

larkox
Copy link
Contributor

@larkox larkox commented Jan 10, 2020 •
edited by hanzei

Summary

Add markdown support to Todos

Ticket Link

Fixes #19

@larkox
Copy link
Contributor Author

larkox commented Jan 10, 2020

In order to add markdown, I have taken a few utils files from webapp, and stripped them down from many things that were not needed for this, and were using other webapp components.

Due to this stripping, some potential bugs may have been created, but should not be the case.

@hanzei hanzei requested review from jwilander and levb January 10, 2020 12:44
@hanzei hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Jan 10, 2020
@larkox
Copy link
Contributor Author

larkox commented Jan 10, 2020

Made changes to use webapp exported PostUtils instead of creating our own markdown parser in my latest commit. Now it is simpler, cleaner, and should work better (since it is using webapp version)

levb
levb approved these changes Jan 10, 2020
View reviewed changes
Copy link
Contributor

@levb levb left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @larkox for researching and fixing this!

The "Add Todo" modal is now behind in that it can use a markdown-aware edit box, I'll file it as a separate issue. We can use it in other plugins as well (Jira, GitHub)

jwilander
jwilander approved these changes Jan 10, 2020
View reviewed changes
Copy link
Contributor

@jwilander jwilander left a comment

Choose a reason for hiding this comment

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

Awesome

@jwilander jwilander removed the 2: Dev Review Requires review by a core committer label Jan 10, 2020
@levb
Copy link
Contributor

levb commented Jan 10, 2020

@DHaussermann I already tested this change minimally (built the plugin and tried some markdown) and it appears to work.

@levb levb mentioned this pull request Jan 10, 2020
Closed
DHaussermann
DHaussermann approved these changes Jan 10, 2020
View reviewed changes
Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

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

Thanks @levb
I also took a look and used various different markdowns. No issues found.
LGTM!
Thanks @larkox !

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Jan 10, 2020
@hanzei hanzei merged commit 00bac63 into mattermost-community:master Jan 13, 2020
@jfrerich jfrerich mentioned this pull request Mar 19, 2020
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@levb levb levb approved these changes

@jwilander jwilander jwilander approved these changes

@DHaussermann DHaussermann DHaussermann approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Todo text should support Markdown
5 participants
@larkox @levb @jwilander @DHaussermann @hanzei