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

update draft to my github branch fix #1317 #1323

Merged
merged 5 commits into from
Sep 3, 2017
Merged

update draft to my github branch fix #1317 #1323

merged 5 commits into from
Sep 3, 2017

Conversation

mattkrick
Copy link
Member

No description provided.

@jordanh
Copy link
Contributor

jordanh commented Sep 1, 2017

Test Template

It:

  • allows me to create Card with Safari browser

@jordanh
Copy link
Contributor

jordanh commented Sep 1, 2017

I'm getting this:

1504308286203 - Origin: Worker (PID 7390)
   [Error] Error: /Users/jordanhusney/Source/Repositories/github.com/Parabol/action.git/src/universal/modules/email/components/Card/Card.js: Cannot find module 'draft-js' from '/Users/jordanhusney/Source/Repositories/github.com/Parabol/action.git/src/universal/modules/email/components/Card'

even if I delete yarn.lock and node_modules...

@mattkrick
Copy link
Member Author

oh grrr, i heard something about yarn not playing kindly with tarballs & non-relative paths. lemme try another way

@mattkrick
Copy link
Member Author

that feeling when finding the bug & fixing it took less time than installing a dependency from github...

@codecov
Copy link

codecov bot commented Sep 1, 2017

Codecov Report

Merging #1323 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1323   +/-   ##
=======================================
  Coverage   56.24%   56.24%           
=======================================
  Files         148      148           
  Lines        2130     2130           
  Branches      304      304           
=======================================
  Hits         1198     1198           
  Misses        784      784           
  Partials      148      148

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 2b15a04...a9dd985. Read the comment docs.

@jordanh
Copy link
Contributor

jordanh commented Sep 2, 2017

Something odd, unrelated to these changes:

image

@jordanh
Copy link
Contributor

jordanh commented Sep 2, 2017

Sorry, went too fast there. That happened when I tried to signout.

@jordanh
Copy link
Contributor

jordanh commented Sep 2, 2017

Hmm, this may be a red herring. I think my .env got smoked (somehow??)

@jordanh
Copy link
Contributor

jordanh commented Sep 2, 2017

Test Cycle 2

It:

  • allows me to create Card with Safari browser

Results: tested on Safari, I still wasn't able to type. Is the package in package.json correct?

@jordanh
Copy link
Contributor

jordanh commented Sep 2, 2017

I gave this another crack. I tested the CSS hack from your PR (facebookarchive/draft-js#1356) and saw that it worked. I also verified that the draft-js version I had contained your fix. However, when I tested with Safari it still wasn't working.

There must be some other component in the tree that's behaving badly...

Were you able to test this an verify it with your local Safari?

@mattkrick
Copy link
Member Author

some versions of safari need a -webkit prefix.
all fixed up & working on dev!

@jordanh
Copy link
Contributor

jordanh commented Sep 3, 2017

Test Cycle 3

It:

  • allows me to create Card with Safari browser

@jordanh
Copy link
Contributor

jordanh commented Sep 3, 2017

Verified! Bravo @mattkrick !!

@jordanh jordanh merged commit b04eb4b into master Sep 3, 2017
@jordanh jordanh deleted the temp-draft-fix branch September 3, 2017 03:52
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.

2 participants