Change dependency: ngx-jodit instead of jodit-angular

[jamesrwelch]

Fixes #735

[pjmonks]

I can't seem to switch back to develop branch due to npm install changing a lot of things on my local build, so can't check this myself. But did this custom toolbar button originally have an icon? I can't remember if it existed before or it was a change I made to the pending PR #572

[pjmonks]

I see the following TypeScript warnings now:

Warning: C:/Code/Mauro/MauroDataMapper/mdm-ui/node_modules/jodit/src/config.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

Warning: C:/Code/Mauro/MauroDataMapper/mdm-ui/node_modules/jodit/src/core/constants.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

Warning: C:/Code/Mauro/MauroDataMapper/mdm-ui/node_modules/jodit/src/types/storage.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

Warning: C:/Code/Mauro/MauroDataMapper/mdm-ui/node_modules/jodit/types/types/storage.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

I think maybe the tsconfig.app.json file could have an excludes list added with these files to remove the warnings, but not sure. See https://stackoverflow.com/questions/57729518/how-to-get-rid-of-the-warning-ts-file-is-part-of-the-typescript-compilation-but for more information perhaps.

[pjmonks]

Found a bug when editing existing HTML content:

1. Pick a catalogue item and edit the description. Set the value through the HTML editor and save.
2. Confirm the description was saved - the catalogue item should reload and you'll see your changes.
3. Now edit the description again. The HTML editor is blank.
4. Switch the Markdown editor option though and the content is there.

Some sort of binding issue it seems, perhaps [(ngModel)] isn't supported in this now Jodit wrapper?

[pjmonks]

The code changes look fine but found some functional issues when testing. Could you review them please?

[pjmonks]

Should we be trying to avoid the ^ symbol for npm package dependencies? The various bots running against the repo to check dependencies seem to want to change them to hard, concrete versions to avoid any version mismatch.

[jamesrwelch]

I've read this: https://docs.renovatebot.com/dependency-pinning/. It all makes sense... I think you're right, we should go with their suggestion and pin dependencies. I've pushed a commit to do this

[jamesrwelch]

I can't seem to switch back to develop branch due to npm install changing a lot of things on my local build, so can't check this myself. But did this custom toolbar button originally have an icon? I can't remember if it existed before or it was a change I made to the pending PR #572

You're right - they've changed the way icons work in Jodit. I couldn't find one I liked in their default set, so I've pushed a commit with a new SVG and a change to code.

[jamesrwelch]

Found a bug when editing existing HTML content:

1. Pick a catalogue item and edit the description. Set the value through the HTML editor and save.
2. Confirm the description was saved - the catalogue item should reload and you'll see your changes.
3. Now edit the description again. The HTML editor is blank.
4. Switch the Markdown editor option though and the content is there.

Some sort of binding issue it seems, perhaps [(ngModel)] isn't supported in this now Jodit wrapper?

Great catch - thank you! I believe I've now done the right thing by binding value instead. This caused issues with the name property, which I've removed and I think this doesn't cause any issues, but could you double-check when you re-test, please?

[jamesrwelch]

I see the following TypeScript warnings now:

Warning: C:/Code/Mauro/MauroDataMapper/mdm-ui/node_modules/jodit/src/config.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

Warning: C:/Code/Mauro/MauroDataMapper/mdm-ui/node_modules/jodit/src/core/constants.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

Warning: C:/Code/Mauro/MauroDataMapper/mdm-ui/node_modules/jodit/src/types/storage.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

Warning: C:/Code/Mauro/MauroDataMapper/mdm-ui/node_modules/jodit/types/types/storage.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

I think maybe the tsconfig.app.json file could have an excludes list added with these files to remove the warnings, but not sure. See https://stackoverflow.com/questions/57729518/how-to-get-rid-of-the-warning-ts-file-is-part-of-the-typescript-compilation-but for more information perhaps.

I don't get these when I upgrade to angular 14, so maybe this is just a temporary thing. There are lots of other warnings then though, so maybe we could have a separate ticket to remove any remaining build warnings once we've completed the upgrade

[jamesrwelch]

@pjmonks Thank you so much for your thorough review - I'm glad we're picking up these issues that would have been difficult to spot amongst a much bigger upgrade.
I believe I've addressed all your issues, so please feel free to re-test. However, although ng serve is working for me, I'm getting a build error running npm run build. I'll look into this next - would be interesting to hear if you have the same problem. In bouncing between branches I've been finding it helpful to do npm cache verify / npm cache clear --false in order to get clean installs on a different branch. I'll be glad when I can stop doing that!

[jamesrwelch]

I'm getting a build error running npm run build. I'll look into this next - would be interesting to hear if you have the same problem. In bouncing between branches I've been finding it helpful to do npm cache verify / npm cache clear --false in order to get clean installs on a different branch. I'll be glad when I can stop doing that!

I've done some more investigation:

• This only occurs during npm run build and not building npm run dist, so it's not a huge problem. It relates to the source maps for debugging TS code.
• It happens for me building develop (recent, and an older version), so it's not a regression
• I don't get the errors in my upgrade to Angular 14, which will be my next PR after this.

So I think we might be ready for re-testing on this one 👍

[pjmonks]

Re-tested and the changes are functioning better now, looks no different using the new Jodit wrapper compared to before 👍

Approved ✔️