highcharts / highcharts-react-native

Other
103 stars 79 forks source link

Using extracted react-native-webview instead of the webview in RN core #8

Closed SaeedZhiany closed 4 years ago

SaeedZhiany commented 5 years ago

Currently, this library is using the RN core's webview component that causes a warning message at runtime.

Screenshot_1563861438

barunprasad commented 5 years ago

This would throw an error with RN 0.60.x.

Screen Shot 2019-08-01 at 10 08 25 PM
sebastianbochan commented 5 years ago

Hi, Thank you for reporting about the problem. I will fix it as soon as possible.

bartvanduinkerken commented 5 years ago

Hi, Any timeline on this fix?

sebastianbochan commented 5 years ago

Hi @bartvanduinkerken, At this moment we do not have ETA for it, but will try to update next week.

bartvanduinkerken commented 5 years ago

Hi @sebastianbochan That would be great :) thank you for your time

SachinSharma2018 commented 5 years ago

@sebastianbochan Any update in this because of this, we are not able to use this library.

SchwSimon commented 5 years ago

@sebastianbochan Same here, we need this update please.

mukeshsharma1201 commented 5 years ago

This would throw an error with RN 0.60.x.

Screen Shot 2019-08-01 at 10 08 25 PM

Facing this same issue.

sebastianbochan commented 5 years ago

Hi, Im still working on this cause of came acrossing a few problems.

Meantime I prepared a solution which works properly in Expo and iOS. It uses react-native-webview and looks like promising.

Any feedback would be really helpful.

webview_app.zip

SchwSimon commented 5 years ago

@sebastianbochan Yes, that's all we need. As stated here facebook.github.io/react-native/docs/webview You just need to add this package github.com/react-native-community/react-native-webview And use the WebView component from this package instead of rn.

Could you push this small update to master please?

sebastianbochan commented 5 years ago

Yes yes I agree, this package is included.

This update is breaking-change, so I would preferer wait for extra feedback from other users.

mukeshsharma1201 commented 5 years ago

Idk maybe I was not able to install it correctly?
I am getting Error: 'dist/highcharts-files/highcharts.js' cannot be loaded as its extension is not registered in assetExts
::1 - - [30/Aug/2019:12:22:28 +0000] "GET /assets/dist/highcharts-files/highcharts.js HTTP/1.1" 404 - "http://localhost:8081/assets/dist/src/index.html?platform=ios&hash=43e650e85e6de95a8ff51b327c36d25d" "Mozilla/5.0 (iPhone; CPU iPhone OS 12_4 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148"

...and a blank white screen.

And I am not using expo.

SchwSimon commented 5 years ago

@sebastianbochan could you please create a branch for this instead of sharing a file?

mukeshsharma1201 commented 5 years ago

@sebastianbochan I have tried using local minified js files with react-native-webview with no luck. This wrapper is much needed. Can you please update on this issue?

sebastianbochan commented 5 years ago

I prepared a beta-release candidate.

Please test the demo, created on separate branch: https://github.com/highcharts/highcharts-react-native/tree/beta-webview

Any feedback will be helpful.

SchwSimon commented 5 years ago

@sebastianbochan Can you fix these 2 errors in your package.json file please.

"errors": [
    "Missing required field: name",
    "Missing required field: version"
]
sebastianbochan commented 5 years ago

@SchwSimon

{
  "main": "node_modules/expo/AppEntry.js",
  "name": "@highcharts/highcharts-react-native",
  "version": "1.0.0",
  "license": "SEE LICENSE IN <LICENSE>",
  "repository": {
    "type": "git",
    "url": "git+https://github.com/highcharts/highcharts-react-native-official.git"
  },
  "keywords": [
    "highcharts",
    "highstock",
    "highmaps",
    "react-native"
  ],
  "devDependencies": {
    "eslint": "^5.16.0",
    "eslint-plugin-react": "^7.12.4",
    "react": "^16.4.0",
    "react-addons-test-utils": "^15.6.2",
    "react-dom": "^16.4.0",
    "react-test-renderer": "^16.4.0",
    "react-unit": "^3.0.0"
  },
  "errors": [
    "Missing required field: name",
    "Missing required field: version"
  ]
}

will be correct?

SchwSimon commented 5 years ago

@sebastianbochan you can remove the "errors" prop, this was just a list of the package.json errors. So basically "name" and "version" props were missing.

You can also validate your package.json file here: http://package-json-validator.com

sebastianbochan commented 5 years ago

@SchwSimon, My mistake, I verified the package.json from other branch.

I added missing fields to the new package.json by commit: https://github.com/highcharts/highcharts-react-native/commit/16767eaed47cc0e212a48a0eacfa5a2a3b78a313

SchwSimon commented 5 years ago

@sebastianbochan Thank you, I just tested the branch, I can't bundle anymore. EDIT: I don't use Expo.

jest-haste-map: Haste module naming collision: react-native
  The following files share their name; please adjust your hasteImpl:
    * <rootDir>\node_modules\react-native\package.json
    * <rootDir>\node_modules\expo\node_modules\react-native\package.json
sebastianbochan commented 5 years ago

@SchwSimon, The main repo is demo expo application, but if you need only Highcharts-react-native package, you should clone only the dist folder and then call npm install or yarn.

SchwSimon commented 5 years ago

@sebastianbochan Could you create another branch for the ones not using Expo, so we can properly install the package?

Also note that even in the /dist package, the entry point main still refers to expo "main": "node_modules/expo/AppEntry.js"

sebastianbochan commented 5 years ago

@SchwSimon Done https://github.com/highcharts/highcharts-react-native/tree/dist-branch

SchwSimon commented 5 years ago

@sebastianbochan Thank you very much. Ok so the package installation and bundling is working fine now. But there is a TypeError, I am using the most basic example:

<HighchartsReactNative
    styles={{}}
    options={{
        series: [{
            data: [1, 2, 3]
        }]
    }} />

grafik

sebastianbochan commented 5 years ago

@SchwSimon, Well I was not able to reproduce this error, any ideas how can I do that?

SchwSimon commented 5 years ago

@sebastianbochan I think it may be due to your node reference. On mount you try to access this.webView which does not exist as you try to get the WebView ref by ref={this._webViewRef}

sebastianbochan commented 5 years ago

@SchwSimon, Will check that, meantime Im working on full-compatiblity with android https://github.com/highcharts/highcharts-react-native/issues/4

markcrockett commented 5 years ago

@sebastianbochan also experiencing this.webView is undefined in DidMount.

this.webView.postMessage( this.serialize(this.props.options, true) );

I was able to reproduce by updating chart config stored in state.

wzhang2 commented 5 years ago

do we have an ETA for the fix?

KacperMadej commented 5 years ago

Hi @wzhang2 We do not have ETA for the fix at this moment. Our main wrapper developer is our of the office and will be back in next week - he might have more detail for the case.

sebastianbochan commented 5 years ago

Im getting back to working on it, hope that tomorrow have a fix for mentioned issues.

BabishanM commented 5 years ago

@sebastianbochan Hi I am facing the same issue

"@highcharts/highcharts-react-native": "^1.0.0", "react": "16.8.6", "react-native": "0.60.5", "react-native-webview": "^7.1.0",

When i tried the same with the provided beta-webview branch

i am getting the below issue Screenshot 2019-09-27 at 4 48 15 PM

To fix the above, I added a condition if (this.webView) { this.webView.postMessage( this.serialize(this.props.options, true) ); }

Screenshot 2019-09-27 at 5 03 27 PM

To fix it, when i try to pass pass source value with html directly,

source={{ html: <html>...</html> }}

I am getting white screen(with no error).

Can you please assist on the same, been stuck up on this for more that 1 week.

Thank you

sebastianbochan commented 5 years ago

Hi @BabishanM , Thank you for really big feedback and advices. I committed your suggestions on my beta-webview branch.

Will be really grateful if you will be able to test it too.

ps. Im waiting for final feedback about Android before final release. (case #4)

BabishanM commented 5 years ago

Hi @sebastianbochan

Thank you for your time to fix this. Can you let us know when will this be reflecting in master? Like an ETA?

Again thank you.

sebastianbochan commented 5 years ago

Hi @BabishanM, On Monday I will ad CDN support and multiple modules, then prepare final beta-release. If all will be correct, I hope that 1 or 2.10 will publish official 2.0 version.

BabishanM commented 5 years ago

Hi @sebastianbochan

It is not working as expected, I am getting blank screen. I have mentioned about it in issue #4 Did i miss something?

SchwSimon commented 5 years ago

@sebastianbochan With your next push, could you also please push a new version to the dist-branch. Thank you.

sebastianbochan commented 5 years ago

@SchwSimon I did it.

Meantiime I also publish new beta-version which supports loading multiple files from file system or CDN (set param useCDN={true}). All is commented in App.js.

Hope that all tests will pass.

Let me know if you came across any problems.

leonascimento commented 5 years ago

@sebastianbochan this is not working yet.

Just a little observation question. Do we need to usehttps here?

https://github.com/highcharts/highcharts-react-native/commit/64eb2814f81c784a48fad5ff91c74f96f4e03b25#diff-0db2095e1ba25d92a152d80a7f75a71fR12

I saw the CDNs links on this page using https. http://code.highcharts.com/

sebastianbochan commented 5 years ago

Hi @leonascimento, Https is not correct supported in this solution, so at this moment the only way (which I found) was skip SSL.

Could you elaborate what kind of errors you have? Still html is printed as string ? If yes, did you check this issue:

SchwSimon commented 5 years ago

@sebastianbochan I get a bundling error because you added entry point ("main") in the package.json.

It works when I remove it.

Also the component renders this: grafik

leonascimento commented 5 years ago

@sebastianbochan yes, I'm seeing just HTML printed as a string. Thank about this link. I will try to import following the tips of this issue.

sebastianbochan commented 5 years ago

So lets wait for the further feedback until tomorrow or end of this week. If all will be fine, I will merge with the current master.

taehajoo commented 5 years ago

I am using the beta webview branch and it has solved my blank screen issue. I am having no problems with it. I hope this gets merged to master soon.

sebastianbochan commented 5 years ago

@taehajoo ios or android?

taehajoo commented 5 years ago

@sebastianbochan Just iOS simulator via Expo. I was getting a blank white screen before using master branch but beta-webview branch fixes the issue.

sebastianbochan commented 5 years ago

The master-branch is broken, so just looking for good solution, but always is the same problem, that it works in a few scenarios and does not work also in a few.

mukeshsharma1201 commented 5 years ago

Any updates on this issue? Can you please merge this in master?

SchwSimon commented 5 years ago

@sebastianbochan We also really need this, clients are waiting and paying for this service without being able to use it because of the issue.

sebastianbochan commented 5 years ago

Regarding to update, in the wrapper I used webkit.postmessage and event to cache it. At this moment it was removed from webview and is still developed / reorganized.

Ref:

@SchwSimon @mukeshsharma1201

I can merge it but updates will be broken. Is it fine for you?