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

Feature/angular 17 #1529

Merged
merged 22 commits into from
Jun 11, 2024
Merged

Feature/angular 17 #1529

merged 22 commits into from
Jun 11, 2024

Conversation

JeltevanBoheemen
Copy link
Contributor

Resolves #1329

A few important notes:

@JeltevanBoheemen JeltevanBoheemen requested a review from ar-jan April 4, 2024 12:46
Copy link
Contributor

@ar-jan ar-jan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I've only tested locally with the JMIG corpus, there everything is functional.

You can address this console warning defaultLabel property is deprecated since 16.6.0, use placeholder instead for primeng multiselect.

Note for Docker use, need to rebuild the containers or webpack complains about Edge version.

@@ -0,0 +1 @@
18.17.1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just 18?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few libraries that are pinned lower than they could be because they require a higher minor verion of node. So the specific pin to 18.17.1 is necessary here.

@lukavdplas
Copy link
Contributor

Reopens #1347. Working on an Ivy distribution of the package to fix it again

That bug can cause some very annoying behaviour (if I recall correctly, it was not limited to visualisation options); since this update does not bring any direct improvements to the user, can we hold off on merging it?

Big version jump for ng2-pdf-viewer. i don't have data to test this. @lukavdplas or @BeritJanssen would you be so kind to check if the pdf viewer still works?

I don't have any PDF data locally; perhaps the test server is the easiest place to check this?

@BeritJanssen
Copy link
Contributor

#1408 depends on a newer Angular version, so I'm afraid we cannot hold this off for too long.

@ar-jan
Copy link
Contributor

ar-jan commented Apr 16, 2024

By the way, can these warnings be ignored or should they be addressed?

warning " > ng2-pdf-viewer@10.0.0" has unmet peer dependency "pdfjs-dist@~2.16.105".
warning " > terser-webpack-plugin@5.3.6" has unmet peer dependency "webpack@^5.1.0".

@ar-jan ar-jan mentioned this pull request Apr 24, 2024
3 tasks
@JeltevanBoheemen
Copy link
Contributor Author

By the way, can these warnings be ignored or should they be addressed?

warning " > ng2-pdf-viewer@10.0.0" has unmet peer dependency "pdfjs-dist@~2.16.105".
warning " > terser-webpack-plugin@5.3.6" has unmet peer dependency "webpack@^5.1.0".

I have added them to the peer dependencies in package.json. Oddly, when doing things directly in the frontend (such as updating browserslists), the warnings still occur. Anyway, safe to ignore.

@JeltevanBoheemen JeltevanBoheemen merged commit 9d2b530 into develop Jun 11, 2024
1 check passed
@JeltevanBoheemen JeltevanBoheemen deleted the feature/angular-17 branch June 11, 2024 14:04
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.

update angular
4 participants