Markdown in OutcomeCards! #640

ndey96 · 2017-01-15T16:39:21Z

This PR is meant to address #300

Opening it early. As of now, editing existing outcome cards is supported. Now just gotta work on editing new ones 💯

Add CircleCI 2.0 beta

jordanh · 2017-01-15T19:24:42Z

Huzzah!

jordanh · 2017-01-22T22:29:39Z

Hey, you're really getting somewhere! Nice work so far!

There are a few things we'll need to work out, aesthetically and interaction-wise:

What padding should we use internally?
Should we get rid of pressing enter to submit? If so, how should cards be submitted?
Should we scale images that are too large? Or, should we clip them?
How shall we handle clicks on links? Should they open a new window? How shall we intermediate the click-to-edit?

What other things should we think about?

ndey96 · 2017-01-22T22:29:45Z

Everything should work properly now, just need to add the proper styles...

jordanh · 2017-01-22T22:29:58Z

Jinx!

ndey96 · 2017-01-22T22:33:47Z

Styles:

I think we can reuse the padding that was used before
want to add hover -> backgroundColor = grey
work-break -> break-word

I think enter to submit should definitely stay and we should also add support for ctrl+enter (right now only shift+enter is supported)
I feel like clicking links should open new tabs, hopefully this markdown library makes that easy
Might also have to look at dealing with super large photos and setting a max width

jordanh · 2017-01-22T22:37:13Z

Awesome! Keep going. Excellent work thus far.

… bug

ndey96 · 2017-01-23T03:37:19Z

@jordanh @ackernaut @mattkrick Most of the styling and logic should be done now :)

Just need to style any img tags within the markdown content to have max-width: 100% but I can't figure out the syntax :(

In other words how would you do this:

.markdown img: {
    max-width: 100%
}

ndey96 · 2017-01-23T03:40:41Z

testing with this markdown: ![image](https://exoticcars.enterprise.ca/etc/designs/exotics/clientlibs/dist/img/homepage/Homepage-Hero-Car.png)
Currently the image will overflow because it is too wide

ackernaut · 2017-01-23T16:14:48Z

@ndey96 this is looking good! Exciting!

A couple of notes on styling:

With the original design I was going with a look where the formatting was essentially the same when just looking at the cards or interacting with them. This helped the cards have less wonky size changes just for interacting with them. I realize that with markdown, the content of the textarea now can be very different than the output so there will probably be some breathing, but overall I would like to try and retain similar padding and line-height etc.
I noticed that some of the Action card styles (yellow cards during a meeting) were removed. Basically the hover/focus state is an extension of the theme of the card, so we need to work that back in considering any updates from point 1 above.
I think the safest way to handle that image ATM is adding this to globalStyles.js but we will need to keep an eye out if that goofs up a style somewhere else (which should be easy to fix):

img: {
    maxWidth: '100%'
  }

jordanh · 2017-01-23T16:53:37Z

@ndey96:

As I said before, great work on this so far. Before you move onto another issue, can you please offer a checklist of what you think needs to get done before you merge?

I'll respond with a check-listed test plan.

When we've checked off the items on both lists, we should be good to merge. Thank you!

…nCards

ndey96 · 2017-01-24T03:55:11Z

Here is what I think needs to be done before a merge:

add styling for wide images
restore styles for yellow cards during meetings
fix any comments that Matt makes @mattkrick

Nice to haves:

clicking links opens new tab (no option in markdown library for this so I'm not sure how to implement without forking the markdown renderer and writing it myself)
supporting ctrl+enter being equivalent to shift+enter

ackernaut · 2017-01-24T17:21:46Z

Ooh I like your list @ndey96 ! I'll pull down and take a gander. And here is a note:

It occurred to me last night what the UI design spec is that I would like to see for card content:

If a user only uses text (no MD formatting) then the textarea and the content should look the same in terms of typography (line-height, font-size) and layout/gutters (padding). Hover states highlight the editable area, simply editing text results in no change of spacing. If a user does add formatting then we can expect the editing state to look different than the final state.

I'm going to test against this spec, and see how it holds up!

ackernaut · 2017-01-24T17:40:18Z

Ok @ndey96 got 4 demos coming atcha!

Action card, production
Action card, with MD

Project card, production
Project card, with MD

You will notice a difference in hover states and text formatting. I would like to retain the seamless feel of the production state for cards, meaning only cards with formatting would have a different output than when editing.

I would also like to retain the same hover states (with the borders, etc.) unless we iterate on that styling. I believe we were using transparent top and bottom borders in order to keep the height the same as when hovering.

ackernaut · 2017-01-24T17:41:42Z

Also, note that the highlight (hover state) coloring is different for action and project cards.

jordanh · 2017-01-25T14:54:44Z

Test plan

It should:

autofocus when new cards are created
render markdown onSubmit (when tab is pressed, or click to blur)
render links that open a new tab when clicked
allow for embedded images, but size them to the card (i.e. not overflow the container)
disallow the pasting of raw html

jordanh · 2017-01-25T14:55:04Z

@ackernaut would you mind giving this a test and some feedback today?

ackernaut · 2017-01-25T17:39:17Z

Taking a gander now.

ackernaut · 2017-01-25T17:55:34Z

This is coming along very nicely!

I checked off a few tests: 1) autofocus new cards 2) render on tab, on blur 3) links open in new tab 4) images do not overflow

As for 5) disallowing the pasting of raw html—when I paste HTML it just renders as text, so it allows pasting, but has no effect. Does that pass?

A few other notes:

I expected the new line to break (I had to press enter twice to get the break), kind of awkward:
- GitHub example
- Action with MD example
Action card editing styles are not the same as Action card hover styles (and they should be, should be simple fix):
- Action prod
- Action with MD
Hard to reproduce, but sometimes the editing styles stick on blur, and not hovering. Not sure what we can do here, but it seems to be a sticky hover state.

Overall, impressed with the quick progress on this!! 👾

ndey96 · 2017-01-26T04:45:55Z

@ackernaut I believe html should be escaped and render as plain text so we don't use dangerouslySetInnerHTML which could open us up to XSS. right @jordanh ?

Also new checklist:

SQUASH newline bug
fix action card editing styles

jordanh · 2017-01-26T04:50:25Z

Yes, html should be escaped. I believe it's working correctly the way you've got it configured now.

I like you're new checklist! Once you made your updates, I'll test again!

ndey96 · 2017-01-26T05:44:34Z

@jordanh @ackernaut Updates made :) Thanks again for the feedback!

ackernaut · 2017-01-26T20:58:32Z

@ndey96 looking good! 1) hover/editing styles are in place 2) newline 🐛 is bye bye!

…nCards

jordanh · 2017-01-26T23:18:20Z

This comment

has many newlines

and a link

ackernaut · 2017-01-26T23:18:23Z

line

The Action list form feels funny with out being able to submit on enter.

jordanh · 2017-01-27T11:07:50Z

src/universal/modules/userDashboard/components/UserActionList/UserActionListItem.js

@@ -60,7 +59,7 @@ class UserActionListItem extends Component {
          <Field
            name={actionId}
            component={OutcomeCardTextarea}
-            doFocus={!content}
+            doSubmitOnEnter


The Action list felt funny without being able to submit on enter:

So the pro user will shift enter to break new lines?

I think that’ll work!

jordanh · 2017-01-27T11:09:05Z

src/universal/components/LinkNewTab/LinkNewTab.js

+import React, {PropTypes} from 'react';
+
+const LinkNewTab = (props) => {
+  const { children, href, title } = props;


@ndey96 I removed the URL-helping here. It didn't sit well with me web-standards wise, and also, GitHub doesn't do any helping so I don't think we should either.

Let's use this is a starting point and if we need to grow something more ornate because our users are struggling, we will.

jordanh · 2017-01-27T11:13:13Z

@mattkrick I think this is good to go, at least well enough for some testing on staging. Would you give the code a once over and also execute the following tests?

Test plan

It should:

Outcome cards on dashboard or meeting:
- autofocus when new cards are created
- render markdown onBlur (when tab is pressed, or click elsewhere)
Action list items:
- autofocus when new cards are created
- render markdown onBlur (when tab is pressed, or click elsewhere)
- render markdown onEnter
rendered links open a new tab when clicked
allow for embedded images, but size them to the card (i.e. not overflow the container)
disallow the pasting of raw html

ackernaut · 2017-01-27T17:41:15Z

So I can still reproduce, at random, cards where the hover state gets stuck. Sometimes it happens with I copy the contents of one card and go to paste into another card or action item. Working on a screen capture, and reliable steps. However I think this can be captured as bug and not necessarily block merge (unless there is an easy fix!).

ackernaut · 2017-01-27T17:51:01Z

So many times it will happen on a first interaction (since I've left the tab and returned) but when I turn on the screen recording the bug decides to hide from me! Haha.

So look at :24 of :30 in this screen recording:
screen recording

I select the content of the card then go to add it to an action item. The hover state sticks and persists. I have to finally go hover on and off after having created an action item. Is this some type of race condition on mouse movement and rendering life cycle?

Shows stuck hover state

ackernaut · 2017-01-27T17:52:49Z

PS. I realize that I need a space between the # and heading to actually format the heading, doh!

jordanh · 2017-01-28T18:05:25Z

@ackernaut yeah, the hover state business is odd.

Looking into the code, the interaction is driven in places above these changes. OutputCard communicates with OutputCardContainer via onMouseEnter and onMouseLeave events. This causes OutputCardContainer to update a state var called hasHover which then will cause the container's component tree to re-render. Why it appears that the onMouseLeave event isn't always being called is unclear.

I think it's possible this bug was pre-existing.

@mattkrick is it ok if I merge this PR? Or, would you like me to delay until you've had a chance to look? I'd like to put it up on staging...

Merge pull request ParabolInc#636 from ParabolInc/circle-2.0

d05e600

Add CircleCI 2.0 beta

jordanh added the pr review label Jan 15, 2017

Support editing existing OutcomeCards with Markdown

b9808ae

ndey96 force-pushed the markdownCards branch from c7ff3be to b9808ae Compare January 15, 2017 16:41

ndey96 added 5 commits January 18, 2017 20:39

Merge branch 'master' into markdownCards

068c3f0

Merge branch 'master' into markdownCards

4ad7f4e

Merge branch 'master' into markdownCards

68d08b8

Markdown in OutcomeCards works for both new and existing cards

d32a8fb

Markdown in OutcomeCards works for both new and existing cards

3fa00fd

ndey96 force-pushed the markdownCards branch from d32a8fb to 3fa00fd Compare January 22, 2017 22:21

Add styles to markdown, remove unused styles, fix archived card click…

7b37d3e

… bug

jordanh and others added 2 commits January 23, 2017 12:04

Merge branch 'markdownCards' of github.com:ndey96/action into markdow…

67c714a

…nCards

Merge branch 'master' into markdownCards

7cd459c

Add styling for wide images

78d67a0

ndey96 force-pushed the markdownCards branch from 67c714a to 78d67a0 Compare January 24, 2017 04:02

Restore card styles that were removed earlier

c583736

Squash newline bug

624e66f

ndey96 force-pushed the markdownCards branch from ae52ea7 to 624e66f Compare January 26, 2017 05:07

Fix up action card edit styles, linting

b484657

jordanh added 2 commits January 26, 2017 17:57

Merge branch 'markdownCards' of github.com:ndey96/action into markdow…

4340809

…nCards

Revert doFocus prop. Preferring bare truthy props.

9a09bdb

Removed LinkNewTab url-helping. Added doEnterOnSubmit.

5edf591

The Action list form feels funny with out being able to submit on enter.

jordanh reviewed Jan 27, 2017

View reviewed changes

jordanh approved these changes Jan 27, 2017

View reviewed changes

jordanh assigned mattkrick Jan 27, 2017

mattkrick approved these changes Jan 30, 2017

View reviewed changes

jordanh merged commit f8977aa into ParabolInc:master Jan 30, 2017

jordanh removed the pr review label Jan 30, 2017

jordanh mentioned this pull request Feb 1, 2017

Markdown added to outcome cards #300

Closed

3 tasks

mattkrick mentioned this pull request Mar 9, 2024

chore: [Snyk] Upgrade react-swipeable-views-core from 0.13.1 to 0.14.0 #9521

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Markdown in OutcomeCards! #640

Markdown in OutcomeCards! #640

ndey96 commented Jan 15, 2017

jordanh commented Jan 15, 2017

jordanh commented Jan 22, 2017

ndey96 commented Jan 22, 2017

jordanh commented Jan 22, 2017

ndey96 commented Jan 22, 2017 •

edited

Loading

jordanh commented Jan 22, 2017

ndey96 commented Jan 23, 2017 •

edited

Loading

ndey96 commented Jan 23, 2017

ackernaut commented Jan 23, 2017 •

edited

Loading

jordanh commented Jan 23, 2017

ndey96 commented Jan 24, 2017 •

edited

Loading

ackernaut commented Jan 24, 2017 •

edited

Loading

ackernaut commented Jan 24, 2017

ackernaut commented Jan 24, 2017

jordanh commented Jan 25, 2017 •

edited

Loading

jordanh commented Jan 25, 2017

ackernaut commented Jan 25, 2017

ackernaut commented Jan 25, 2017 •

edited

Loading

ndey96 commented Jan 26, 2017 •

edited

Loading

jordanh commented Jan 26, 2017

ndey96 commented Jan 26, 2017 •

edited

Loading

ackernaut commented Jan 26, 2017

jordanh commented Jan 26, 2017 •

edited

Loading

ackernaut commented Jan 26, 2017 •

edited

Loading

jordanh Jan 27, 2017

ackernaut Jan 27, 2017

ackernaut Jan 27, 2017

jordanh Jan 27, 2017

jordanh commented Jan 27, 2017 •

edited

Loading

ackernaut commented Jan 27, 2017

ackernaut commented Jan 27, 2017 •

edited

Loading

ackernaut commented Jan 27, 2017

jordanh commented Jan 28, 2017

Markdown in OutcomeCards! #640

Markdown in OutcomeCards! #640

Conversation

ndey96 commented Jan 15, 2017

jordanh commented Jan 15, 2017

jordanh commented Jan 22, 2017

ndey96 commented Jan 22, 2017

jordanh commented Jan 22, 2017

ndey96 commented Jan 22, 2017 • edited Loading

jordanh commented Jan 22, 2017

ndey96 commented Jan 23, 2017 • edited Loading

ndey96 commented Jan 23, 2017

ackernaut commented Jan 23, 2017 • edited Loading

jordanh commented Jan 23, 2017

ndey96 commented Jan 24, 2017 • edited Loading

ackernaut commented Jan 24, 2017 • edited Loading

ackernaut commented Jan 24, 2017

ackernaut commented Jan 24, 2017

jordanh commented Jan 25, 2017 • edited Loading

Test plan

jordanh commented Jan 25, 2017

ackernaut commented Jan 25, 2017

ackernaut commented Jan 25, 2017 • edited Loading

ndey96 commented Jan 26, 2017 • edited Loading

jordanh commented Jan 26, 2017

ndey96 commented Jan 26, 2017 • edited Loading

ackernaut commented Jan 26, 2017

jordanh commented Jan 26, 2017 • edited Loading

ackernaut commented Jan 26, 2017 • edited Loading

jordanh Jan 27, 2017

Choose a reason for hiding this comment

ackernaut Jan 27, 2017

Choose a reason for hiding this comment

ackernaut Jan 27, 2017

Choose a reason for hiding this comment

jordanh Jan 27, 2017

Choose a reason for hiding this comment

jordanh commented Jan 27, 2017 • edited Loading

Test plan

ackernaut commented Jan 27, 2017

ackernaut commented Jan 27, 2017 • edited Loading

ackernaut commented Jan 27, 2017

jordanh commented Jan 28, 2017

ndey96 commented Jan 22, 2017 •

edited

Loading

ndey96 commented Jan 23, 2017 •

edited

Loading

ackernaut commented Jan 23, 2017 •

edited

Loading

ndey96 commented Jan 24, 2017 •

edited

Loading

ackernaut commented Jan 24, 2017 •

edited

Loading

jordanh commented Jan 25, 2017 •

edited

Loading

ackernaut commented Jan 25, 2017 •

edited

Loading

ndey96 commented Jan 26, 2017 •

edited

Loading

ndey96 commented Jan 26, 2017 •

edited

Loading

jordanh commented Jan 26, 2017 •

edited

Loading

ackernaut commented Jan 26, 2017 •

edited

Loading

jordanh commented Jan 27, 2017 •

edited

Loading

ackernaut commented Jan 27, 2017 •

edited

Loading