brtnshrdr / angular2-hotkeys

Keyboard shortcuts for Angular 2 apps
MIT License
203 stars 95 forks source link

Adopt to new mousetrap types #135

Closed BenSL closed 3 years ago

BenSL commented 4 years ago

With the new @types/1.6.4 angular2-hotkeys is broken. This should fix it.

nicolasdehouck commented 4 years ago

thanks

feitzi commented 4 years ago

Please approve this PR!

RelentlessResults commented 4 years ago

+1

nodulusteam commented 4 years ago

yes please. I've resolved this issue temporarily by using yarn resolutions.

    "resolutions": {   
        "@types/mousetrap": "1.6.3"
    },
roland-vachter commented 4 years ago

+1

orhanmustafa commented 4 years ago

same

exbachi commented 4 years ago

You saved angular from elimination

falexandre commented 4 years ago

+1

euivanw commented 4 years ago

+1

XeL077 commented 4 years ago

HI! YEEESSSSS, give my this fix!

ramonfelizetti commented 4 years ago

+1

thiagomaquito commented 4 years ago

+1

joaomaas commented 4 years ago

+1

fe-telles commented 4 years ago

+1

rafaelmatiello commented 4 years ago

+1

filipermiguel commented 4 years ago

+1

vvnc commented 4 years ago

Is there a way to fix this locally via npm the way the CI could build it?

euivanw commented 4 years ago

@vvnc I've used the tip from @nodulusteam and it worked for me, at least, as a workaround!

https://github.com/brtnshrdr/angular2-hotkeys/pull/135#issuecomment-698639561

vvnc commented 4 years ago

@vvnc I've used the tip from @nodulusteam and it worked for me, at least, as a workaround!

#135 (comment)

Does this require npm-force-resolutions package?

BenSL commented 4 years ago

@vvnc use package-lock or shrinkwrap with npm ci command.

JonathanOldenburg commented 4 years ago

+1

roland-vachter commented 4 years ago

The question is: when will this be merged? So many people needs this ...

kunaljain90 commented 4 years ago

No need to add resolution incase of npm. You can add "@types/mousetrap": "1.6.3" to your dependencies explicitly till this PR is merged.

dependencies: { "@types/mousetrap": "1.6.3" }

Daniel-Anjos commented 4 years ago

+1

shah20 commented 4 years ago

+1

yevgeniy-belov commented 4 years ago

No need to add resolution incase of npm. You can add "@types/mousetrap": "1.6.3" to your devDependencies explicitly till this PR is merged.

devDependencies: { "@types/mousetrap": "1.6.3" }

Didn't work for me. Angular 10.0.12

roland-vachter commented 4 years ago

No need to add resolution incase of npm. You can add "@types/mousetrap": "1.6.3" to your devDependencies explicitly till this PR is merged. devDependencies: { "@types/mousetrap": "1.6.3" }

Didn't work for me. Angular 10.0.12

you should add mousetrap@1.6.3 to dependencies as well

JamesWPritchett commented 4 years ago

No need to add resolution incase of npm. You can add "@types/mousetrap": "1.6.3" to your devDependencies explicitly till this PR is merged. devDependencies: { "@types/mousetrap": "1.6.3" }

Didn't work for me. Angular 10.0.12

you should add mousetrap@1.6.3 to dependencies as well

That works great right up until your build system runs "npm install" and installs 1.6.4, ignoring your hardcoded dependency version.

BenSL commented 4 years ago

@wittlock @brtnshrdr @isaacplmann @AnBauer @nlmueng @DennisRoam pls merge this request and make a new release. i would do by myself if i had the rights to publish. :)

blake-haas commented 4 years ago

@wittlock @brtnshrdr @isaacplmann @AnBauer @nlmueng @DennisRoam Please merge this for new release.

akashlinasayed commented 4 years ago

I am trying to implement "ctrl" and "+" using Hotkeys and it doesn't work (ctrl+- works fine). Is this anything to do with mousetrap?

export class MyClass { zoomInHotkey: Hotkey; constructor(private readonly hotkeys: HotkeysService) { this.zoomInHotkey = new Hotkey(['ctrl++'], () => this.invokeMyZoomInFunction(), ['input', 'textarea', 'select']); //doesn't invoke my zoom in function } }

gustavoraphael commented 4 years ago

+1

SvyatoslavZhuravlev commented 4 years ago

+1

yevgeniy-belov commented 4 years ago

@wittlock @brtnshrdr @isaacplmann @AnBauer @nlmueng @DennisRoam, Is there any reason why this PR can't be merged? What is missing?

mikejoseph23 commented 4 years ago

I am also waiting for a merge to happen. In the meantime, I've tried the work-arounds suggested in this thread and none of them have worked. I've tried:

None of these attempts yielded a successful build and gave me the same error: ERROR in node_modules/angular2-hotkeys/lib/hotkeys.service.d.ts:10:16 - error TS2304: Cannot find name 'MousetrapInstance'.

The only way that this gets fixed is if I do exactly what this pull request would do, which is go into my /node_modules/angular2-hotkeys/lib/hotkeys.service.d.ts and add the following line to the imports: import { MousetrapInstance } from 'mousetrap';

Once I do that, a build succeeds. This isn't ideal because since obviously node_modules aren't in source control, I have to open up the file on my build server and make the same change for it to work. Since my build doesn't clean the build directory each time, this works for now, but the fix needs to be repeated any time I do a clean build.

Hope my post saves someone else some time.

mikejoseph23 commented 4 years ago

I was wrong about my CI build being OK. The last build I ran got rid of my change so every time I want to build, it can potentially break. I added this hack to my build process: After my npm install runs (or yarn in my case), I run the following powershell script prior to running my Angular build:

(Get-Content .\node_modules\angular2-hotkeys\lib\hotkeys.service.d.ts).replace("import 'mousetrap';", "import {MousetrapInstance} from 'mousetrap';") | Set-Content .\node_modules\angular2-hotkeys\lib\hotkeys.service.d.ts

It's ugly but it works for now.

lazar-vuckovic commented 3 years ago

I was wrong about my CI build being OK. The last build I ran got rid of my change so every time I want to build, it can potentially break. I added this hack to my build process: After my npm install runs (or yarn in my case), I run the following powershell script prior to running my Angular build:

(Get-Content .\node_modules\angular2-hotkeys\lib\hotkeys.service.d.ts).replace("import 'mousetrap';", "import {MousetrapInstance} from 'mousetrap';") | Set-Content .\node_modules\angular2-hotkeys\lib\hotkeys.service.d.ts

It's ugly but it works for now.

Until the PR gets merged, you can use npm-force-resolutions and pin the version down to "resolutions": { "@types/mousetrap": "1.6.3" },

malteboettcher commented 3 years ago

When will this be merged?

mikejoseph23 commented 3 years ago

@gszymonik Thanks for approving this! Who is responsible for merging this?

KGDI01 commented 3 years ago

The workaround using @mikejoseph23 suggestion above worked for me.

Edit "node_modules/angular2-hotkeys/lib/hotkeys.service.d.ts"

Comment out / remove: import 'mousetrap';

Add: import { MousetrapInstance } from 'mousetrap';

d3-vitalii-baziuk commented 3 years ago

@BenSL pls close your PR. It's resolved in https://www.npmjs.com/package/angular2-hotkeys v2.3.1

Coffee-Tea commented 3 years ago

It's resolved in https://www.npmjs.com/package/angular2-hotkeys v2.3.1

swapnil28verma commented 3 years ago

I'm facing this issue again in v2.3.2. Following imports are being used - "angular2-hotkeys": "^2.3.2", "@types/mousetrap": "1.6.3",

EDIT: Never mind, figured it out. Turns out the explicit import I had of @types/mousetrap was causing the issues, because it downloaded an older version of the dependency which is no longer compatible with angular2-hotkeys. Once I removed that dependency from package.json (also deleted package-lock.json, just for good measure) and re-ran npm install, it downloaded the correct versions, and builds fine now

dineshkumarv-ict commented 1 year ago

Please approve this PR!

Coffee-Tea commented 1 year ago

@dineshkumarv-ict This adjustment is done a long time ago, please use the latest version.