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

Markdown summary #794

Merged
merged 8 commits into from
Mar 22, 2017
Merged

Markdown summary #794

merged 8 commits into from
Mar 22, 2017

Conversation

ndey96
Copy link
Contributor

@ndey96 ndey96 commented Mar 10, 2017

This addresses #728

It should:

  • render markdown in the meeting summary
  • render markdown in email summary

@jordanh @mattkrick @ackernaut

@codecov
Copy link

codecov bot commented Mar 10, 2017

Codecov Report

Merging #794 into master will decrease coverage by 0.08%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #794      +/-   ##
==========================================
- Coverage   63.92%   63.83%   -0.09%     
==========================================
  Files         109      110       +1     
  Lines        1386     1391       +5     
==========================================
+ Hits          886      888       +2     
- Misses        500      503       +3
Impacted Files Coverage Δ
...rc/universal/modules/email/components/Card/Card.js 14.28% <0%> (-1.1%)
src/universal/components/LinkNewTab/LinkNewTab.js 50% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc889e2...f72cb75. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 10, 2017

Codecov Report

Merging #794 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #794      +/-   ##
==========================================
- Coverage   58.71%   58.68%   -0.03%     
==========================================
  Files         121      122       +1     
  Lines        1659     1663       +4     
==========================================
+ Hits          974      976       +2     
- Misses        685      687       +2
Impacted Files Coverage Δ
...rc/universal/modules/email/components/Card/Card.js 15.38% <ø> (ø) ⬆️
src/universal/components/LinkNewTab/LinkNewTab.js 50% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 509913c...529e66c. Read the comment docs.

@ndey96
Copy link
Contributor Author

ndey96 commented Mar 10, 2017

Meeting summary:
screen shot 2017-03-09 at 11 31 59 pm

@ndey96
Copy link
Contributor Author

ndey96 commented Mar 10, 2017

Anyone know a good way to test whether the emails render markdown correctly? The meetingSummary and summaryEmail both use the same class to render so there should be no problem (Card.js)

@jordanh
Copy link
Contributor

jordanh commented Mar 10, 2017

@ndey96, yes! Check out src/server/worker.js and look for emailSSR. If you change the function it calls, you can test your email...

@ndey96
Copy link
Contributor Author

ndey96 commented Mar 12, 2017

@jordanh Here is what the email looks like:
screen shot 2017-03-12 at 12 37 21 am

@@ -8,6 +8,7 @@ import {DragDropContext as dragDropContext} from 'react-dnd';
import HTML5Backend from 'react-dnd-html5-backend';

const NewTeam = (props) => {
console.log(props);
Copy link
Member

Choose a reason for hiding this comment

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

let's nuke this

Copy link
Member

Choose a reason for hiding this comment

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

@ndey96 any update on this?

@@ -19,7 +20,7 @@ const NewTeam = (props) => {
NewTeam.propTypes = {
dispatch: PropTypes.func.isRequired,
params: PropTypes.shape({
newOrg: PropTypes.object
newOrg: PropTypes.string
Copy link
Member

Choose a reason for hiding this comment

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

good catch!

@@ -53,6 +55,9 @@ const Card = (props) => {
};
}

const markdownCustomComponents = {
Copy link
Member

Choose a reason for hiding this comment

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

let's move this outside the component since it'll never change & then we can reuse that same object for all the cards.

@ndey96
Copy link
Contributor Author

ndey96 commented Mar 22, 2017

@mattkrick made the changes you requested :)

@mattkrick mattkrick merged commit c00f56f into master Mar 22, 2017
@jordanh jordanh removed the pr review label Mar 22, 2017
@mattkrick
Copy link
Member

awesome!

@mattkrick mattkrick deleted the markdownSummary branch March 22, 2017 02:07
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.

3 participants