nativescript-community / nativescript-drawingpad

:pencil: NativeScript plugin to provide a way to capture any drawing (signatures are a common use case) from the device
Apache License 2.0
90 stars 32 forks source link

NS 3.0 - Update for breaking changes #17

Closed emmanuel128 closed 7 years ago

emmanuel128 commented 7 years ago

screenshot_1494128098

emmanuel128 commented 7 years ago
{
  "description": "**************",
  "license": "SEE LICENSE IN <your-license-filename>",
  "readme": "*******",
  "repository": "<fill-your-repository-here>",
  "nativescript": {
    "id": "*********",
    "tns-android": {
      "version": "3.0.0"
    }
  },
  "dependencies": {
    "nativescript-barcodescanner": "^2.3.3",
    "nativescript-camera": "^3.0.0",
    "nativescript-drawingpad": "^1.1.2",
    "nativescript-drop-down": "^3.0.0",
    "nativescript-loading-indicator": "^2.2.2",
    "nativescript-orientation": "^1.5.5",
    "nativescript-push-notifications": "^0.1.2",
    "nativescript-telerik-ui": "^2.0.1",
    "nativescript-theme-core": "^1.0.3",
    "nativescript-timedatepicker": "^1.1.1",
    "nativescript-toast": "^1.4.5",
    "nativescript-vibrate": "^1.1.2",
    "tns-core-modules": "^3.0.0"
  },
  "devDependencies": {
    "babel-traverse": "6.21.0",
    "babel-types": "6.21.0",
    "babylon": "6.14.1",
    "lazy": "1.0.11"
  }
}
bradmartin commented 7 years ago

The breaking changes with 3.0 require some changes to this plugin. Here is the migration guide https://github.com/NativeScript/NativeScript/wiki/Migrating-Plugins-to-NativeScript-3.0 and you'll see here (https://github.com/bradmartin/nativescript-drawingpad/blob/master/drawingpad.android.ts#L2) we use one of the obsolete modules that have been dropped with 3.0.

I don't have time right now to do the update - but would very much appreciate a PR and I can assist and answer any questions if you want to try.

nraboy commented 7 years ago

+1

ferminmoli commented 7 years ago

+1

farzeni commented 7 years ago

Hi all, Today I spent a couple of hours trying to migrate this plugin to NS 3.0. Android side seems to be working. I cannot say anything about iOS side, but from what i read in the code it shouldn't be affected by any breaking change.

However I wasn't able to get the demo working and that's why i'm not pushing a PR.

When i run npm run setup it explodes with:

> nativescript-drawingpad@1.1.3 build /home/fabri/Projects/nativescript-drawingpad
> tsc

demo/node_modules/tns-core-modules/tns-core-modules.d.ts(5,5): error TS2300: Duplicate identifier '"audio"'.
demo/node_modules/tns-core-modules/tns-core-modules.d.ts(5,14): error TS2300: Duplicate identifier '"beacon"'.
demo/node_modules/tns-core-modules/tns-core-modules.d.ts(5,24): error TS2300: Duplicate identifier '"cspreport"'.
demo/node_modules/tns-core-modules/tns-core-modules.d.ts(5,37): error TS2300: Duplicate identifier '"download"'.
demo/node_modules/tns-core-modules/tns-core-modules.d.ts(5,49): error TS2300: Duplicate identifier '"embed"'.
demo/node_modules/tns-core-modules/tns-core-modules.d.ts(5,58): error TS2300: Duplicate identifier '"eventsource"'.
demo/node_modules/tns-core-modules/tns-core-modules.d.ts(5,73): error TS2300: Duplicate identifier '"favicon"'.
demo/node_modules/tns-core-modules/tns-core-modules.d.ts(5,84): error TS2300: Duplicate identifier '"fetch"'.
....

I tried the plugin in another project and it does what it should.

The forked repo is here: https://github.com/farzeni/nativescript-drawingpad maybe someone can give me an advise on what is wrong with demo project configuration.

ah, thanks brad (and others) for all the work you shared on NS. You saved me from a lot of headaches ;)

nraboy commented 7 years ago

Hey @farzeni,

I tried out your fork. I needed to make a few changes to make it work for Android. For example, in drawingpad-common.ts I had to move these two lines above the class:

export const penColorProperty = new Property<DrawingPadCommon, string>({name: "penColor", defaultValue: '#000000'});
export const penWidthProperty = new Property<DrawingPadCommon, string>({name: 'penWidth', defaultValue: '3'});

This prevents an error at build time of the plugin. I also had to alter the tsconfig.ts file, changing this line:

"node_modules/tns-core-modules/tns-core-modules.d.ts",

After that, it worked fine on Android. However, it does not work on iOS and it shows no error logs. I believe iOS detects that a drawing pad is there, but it doesn't pick anything up when I try to draw.

Any ideas? Maybe @bradmartin ?

farzeni commented 7 years ago

Hi @nraboy,

I had a similar issue on android side caused by old _createUI method, not returning the native view.

doc says: The createUI() method is renamed to createNativeView(). It should now return a native view instance instead of setting it locally. https://github.com/NativeScript/NativeScript/blob/master/Modules30Changes.md#view

On iOS side the native view is initialized in constructor, maybe returning the view there or adding createNativeView method returning this._ios will solve the issue.

farzeni commented 7 years ago

Hi again @nraboy,

Your point on tnsconfig.json was the reason I wasn't able to build the demo project.

Then, it is right that the property declaration is below the class, because it needs it for the register method. The wrong thing was the property declaration inside the class, which has to be just a dummy declaration for ts compiler. Then android class was missign getDefault and setNative method, so it wasn't possibile to change color and width property.

However, now demo app builds correctly on android and changing color and width works properly. Another test to my fork is welcome.

I also added createNativeView method in ios, which maybe was the cause of the problem on that side.

ferminmoli commented 7 years ago

I've just tested your fork. The demo project isn't working for me. I'm attaching an screenshot.

screenshot_2017-05-15-11-03-19

bradmartin commented 7 years ago

Is the fork using TS? Did the files transpile?

On Mon, May 15, 2017, 9:07 AM Fermín Molinuevo notifications@github.com wrote:

I've just tested your fork. The demo project isn't working for me. I'm attaching an screenshot.

[image: screenshot_2017-05-15-11-03-19] https://cloud.githubusercontent.com/assets/10004370/26061512/86640666-395e-11e7-9f14-3e3e4fa76625.png

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bradmartin/nativescript-drawingpad/issues/17#issuecomment-301485788, or mute the thread https://github.com/notifications/unsubscribe-auth/AFulhPIPYe4Q07KWKZl_iBBW_xbU3KeCks5r6FwLgaJpZM4NS_2L .

ferminmoli commented 7 years ago

Yes @bradmartin , the fork is currently using TS and is also transpiling.

bradmartin commented 7 years ago

Okay, the error is descriptive enough. Failed to find module "main-view-model" so check the path to that module and make sure you have the ouput .js and it should run at least. I'm not following along too closely with this just yet 😄 but the issue you have should be fixed by making sure the main-view-model.js is in the project and located at the correct path to resolve.

ferminmoli commented 7 years ago

I've just fixed "Failed to find module "main-view-model" but now I'm getting another error. Error Building UI from XML, Drawing pad element was not found.

nraboy commented 7 years ago

@farzeni,

Your latest commit fixes the build errors for me, but it still does not work on iOS. No errors, but nothing is drawn to the screen. This is all foreign to me so I can't contribute, but I'm happy to test whatever you do.

Thanks,

farzeni commented 7 years ago

@nraboy,

I blindly patched ios version, looking to other plugins already migrated like the floating-button one. Maybe you can give it a try and see if something gets better (but hey, this is a blind patch so don't be too hopeful ;) )

@ferminmoli

I noticed that 'npm run setup` sets the plugin path in demo package.json to the full path on my machine. I committed it by mistake. You can try to pull again and from the root of the plugin run:

$ npm run clean
$ npm run setup
$ cd demo
$ tns run android

Let me know if it solves your issue.

ferminmoli commented 7 years ago

@farzeni I'm having same error.

86640666-395e-11e7-9f14-3e3e4fa76625

bradmartin commented 7 years ago

@farzeni - just checking in to see what's left with the update. Thanks for working on this everyone 🤗

cevarief commented 7 years ago

I tried @farzeni fork, built successfully. On IOS it does not draw anything. On Android throwing a bunch of error :

ERROR Error: Uncaught (in promise): TypeError: Could not load view for: DrawingPad.Error: com.tns.NativeScriptExcept ion: Failed to find module: "ui/core/proxy", relative to: app/tns_modules/ JS: com.tns.Module.resolvePathHelper(Module.java:146) JS: com.tns.Module.resolvePath(Module.java:55) JS: com.tns.Runtime.callJSMethodNative(Native Method) JS: com.tns.Runtime.dispatchCallJSMethodNative(Runtime.java:1021) JS: com.tns.Runtime.callJSMethodImpl(Runtime.java:903) JS: com.tns.Runtime.callJSMethod(Runtime.java:890) JS: com.tns.Runtime.callJSMethod(Runtime.java:874)

bradmartin commented 7 years ago

Thanks for all the work here everyone. I'm publishing 2.0.0 in a few minutes with NS 3.0 support.