Improved Angular compatibility #2229

JBBianchi · 2023-12-14T15:54:13Z

In the current stage, @jsonforms/angular-material still relies on @angular/flex-layout.

This package has been deprecated and isn't compatible with Angular v16+, therefore @jsonforms/angular-material would be stuck to Angular v15.

This PR removes the dependency to @angular/flex-layout by replacing the attributes with plain CSS instead.

Doing so, @jsonforms/angular-material becomes compatible with Angular v16 and v17 (at first sight).

For testing purposes, I forced upgrade the jsonforms-angular-seed projects to both v16 and v17:

I tried building the packages as submitted in this PR and running them in the above projects and it seems to work fine (it displays, I can remove one of the two items and I can update a text field. I didn't test more than that).

Related to

Support Angular w/ Material v16 #2138

…x layout

netlify · 2023-12-14T15:54:18Z

✅ Deploy Preview for jsonforms-examples ready!

Name	Link
🔨 Latest commit	`44244a0`
🔍 Latest deploy log	https://app.netlify.com/sites/jsonforms-examples/deploys/65953e1ea605900007043ed4
😎 Deploy Preview	https://deploy-preview-2229--jsonforms-examples.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

JBBianchi · 2023-12-14T15:54:54Z

(Follow up of #2212)

JBBianchi · 2023-12-14T17:26:53Z

~~I might have messed up the pnpm-lock.yaml ...~~

~~Is there a way for pnpm to use Angular v15 even if the package is compatible with higher versions ? (compile vs usage)~~

Edit: I went with pnpm.overrides

gexclaude · 2023-12-15T07:51:21Z

@JBBianchi really appreciate your efforts!

coveralls · 2023-12-15T09:00:54Z

coverage: 83.078%. remained the same
when pulling 44244a0 on JBBianchi:angular-v15-no-flex-layout
into a7d3268 on eclipsesource:master.

JBBianchi · 2023-12-15T10:13:02Z

We could actually use Angular v16 for the build process, which will allow to be compliant with #2224 .

Should I add this to this PR as well ? It's basically just changing the pnpm.overrides and removing the entryComponents config from the tests so the impact is pretty low.

@sdirix @lucas-koehler WDYT ?

Edit: FYI, in the meanwhile, I pushed those changes to https://github.com/JBBianchi/jsonforms/tree/angular-v16 so you can compare both branches. (the pipeline runs successfully https://github.com/JBBianchi/jsonforms/actions/runs/7220751941 )

lucas-koehler · 2023-12-15T10:40:25Z

Hi @JBBianchi,
first of all thanks for the PR and your continued effort on improving the angular renderer set ❤️
I see no harm in using Angular 16 for the build process already. So gladly go ahead with that :)

JBBianchi · 2023-12-15T10:52:49Z

Hi @JBBianchi, first of all thanks for the PR and your continued effort on improving the angular renderer set ❤️ I see no harm in using Angular 16 for the build process already. So gladly go ahead with that :)

For ng16, I'll hold my horses because the example doesn't seem to run. I'll need to investigate a bit more even though all lights seemed green at first.

Edit: For the record, I have no idea why but there is a "glitch" in the rollup output of the example with Angular 16. In @angular/core, a QueryList is declared as:

class QueryList {
    static { Symbol.iterator; }
    /**
     * Returns `Observable` of `QueryList` notifying the subscriber of changes.
     */
    get changes() {
        return this._changes || (this._changes = new EventEmitter());
    }
...

But in the rollup output, the static bit is messed up:

class QueryList {
    s }
    /**
    * Returns `Observable` of `QueryList` notifying the subscriber of changes.
    */
    get changes() {
        return this._changes || (this._changes = new EventEmitter());
    }
...

Edit 2: the fix was simply to update rollup, I should have thought about that faster.

JBBianchi · 2023-12-15T12:22:13Z

@lucas-koehler @sdirix the PR should be ready for review.

lucas-koehler

Hi thanks again for the PR ❤️
The changes already look pretty good to me :)
I have some minor comments/questions inline.

packages/angular-material/src/controls/range.renderer.ts

packages/angular-material/src/layouts/group-layout.renderer.ts

packages/angular-material/test/group-layout.spec.ts

packages/angular-material/test/horizontal-layout.spec.ts

packages/angular-material/tsconfig.spec.json

- readded angularCompilerOptions in tests tsconfig - removed empty imports array in tests - applied flex to divs only in group-layout renderer - removed html comment in range renderer

lucas-koehler

Hi the code changes LGTM. I had a look at the example. It looked pretty good to me. I only noticed that the group layout is now a bit cramped due to missing padding.

packages/angular-material/src/layouts/group-layout.renderer.ts

lucas-koehler · 2023-12-20T10:20:47Z

@JBBianchi Thanks for the update. Do you know whether the code will still work for Angular 15 when it's built with Angular 16 ?
If yes that's fine for me :) Otherwise, we can also remove compatibility to Angular 15.

lucas-koehler

In the seed, I tested the changes with 15, 16 and 17. The seed did not compile with 15 but worked with 16 and 17. Thus, I suggest removing compatibility to 15. It is end of life in less than half a year anyway.

Also I noticed another bug, the packages/angular-material/src/controls/range.renderer.ts does no longer have functioning data binding. I think this might already have been introduced with the Angular 15 support PR when the value and onChange properties were removed in packages/angular-material/src/controls/range.renderer.ts

JBBianchi · 2023-12-22T16:34:56Z

In the seed, I tested the changes with 15, 16 and 17. The seed did not compile with 15 but worked with 16 and 17. Thus, I suggest removing compatibility to 15. It is end of life in less than half a year anyway.

Also I noticed another bug, the packages/angular-material/src/controls/range.renderer.ts does no longer have functioning data binding. I think this might already have been introduced with the Angular 15 support PR when the value and onChange properties were removed in packages/angular-material/src/controls/range.renderer.ts

Thanks for testing in the seed v15, that's unfortunate but indeed, support could be dropped.

I'll try to have look during next week if I have some time.

JBBianchi · 2023-12-23T16:10:51Z

It seems indeed that a library built with ng 16 is not compatible with ng 15 because the signature of the @Input() directive changed.

For the range control, the binding of the change event was "cleaned out" by mistake in this commit.

Next commit will remove compatibility with ng 15 and rebind a the event to the range control.

…inding

…nchi/jsonforms into angular-v15-no-flex-layout

lucas-koehler

Hi @JBBianchi thanks for the updates :) Changes look pretty good to me now. I have just one question regarding the pnpm overrides inline.
I also updated the lock file.

lucas-koehler · 2024-01-02T09:35:25Z

package.json

+    "overrides": {      
+      "@angular/animations": "^16.0.0",
+      "@angular/cdk": "^16.0.0",
+      "@angular/common": "^16.0.0",
+      "@angular/core": "^16.0.0",
+      "@angular/forms": "^16.0.0",
+      "@angular/material": "^16.0.0",
+      "@angular/platform-browser": "^16.0.0",
+      "@angular/router": "^16.0.0",      
+      "@angular-devkit/build-angular": "^16.0.0",
+      "@angular-eslint/eslint-plugin": "^16.0.0",
+      "@angular-eslint/eslint-plugin-template": "^16.0.0",
+      "@angular-eslint/schematics": "^16.0.0",
+      "@angular-eslint/template-parser": "^16.0.0",
+      "@angular/compiler": "^16.0.0",
+      "@angular/compiler-cli": "^16.0.0",
+      "@angular/platform-browser-dynamic": "^16.0.0",
+      "@ngtools/webpack": "^16.0.0",
+      "ng-packagr": "^16.0.0",
+      "zone.js": "^0.13.0"
+    }


Do we still need these overrides now that we removed Angular 15 compatibility?

I'm not 100% sure. If I'm not mistaken, as the packages declare a compatibility with Angular 16 and 17 (and maybe 18 in the future), pnpm will install the highest version compatible by default, so v17. So we need the overrides to say "build with v16 even if the lib is compatible with v17".

Can we use Angular 16 in the devDependencies and not 16 || 17? As we don't bundle Angular the peerDependencies version is the important one for the consumer and could still be 16 || 17 even if the devDependencies only declare 16.

@sdirix thanks for your input, that's better indeed.

This reverts commit fe5f906.

Update the pnpm lock file and override vue and @vue/compiler-sfc dependencies to ~3.3.13 because vue-vanilla tests currently fail when updating to vue 3.4.x.

lucas-koehler

Hi @JBBianchi , the PR looks good to me now. I updated all dependencies and added overrides for vue because its latest version doesn't work. I added issue #2234 to fix this later.

Thanks again for your effort on the Angular packages ❤️

stanisev · 2024-04-29T12:50:52Z

packages/angular-material/src/other/master-detail/master.ts

@@ -140,9 +143,6 @@ export const removeSchemaKeywords = (path: string) => {
        top: 0;
        right: 0;
      }
-      .hide {
-        display: none;


Why is this deleted?

It might be an error when factoring out @angular/flex-layout with [ngStyle]="{ display: hidden ? 'none' : '' }" instead of [fxHide]="hidden". Do you encounter any problem ?

Edit:
It is mistake indeed. The delete button should be displayed only when an item is hovered and hidden (via the hide class) by default. Removing the hide class made the delete button always visible.

Yeah right, that's why I am writing. The delete button is always shown. We created a custom listWithDetail renderer to counter this problem. Is there a better solution? Or can you restore the deleted CSS class?

I'm not a maintainer of this repo but I'd assume you can open an issue explaining the problem and them submit a PR to solve it. Then maintainers can maybe accept the PR and push it in the next release.

Happy to review and merge a PR

I've created the issue: #2327
Should you give me permission in order to open a MR?

@stanisev Thanks for creating the issue :) You can gladly open a MR anytime you want.

@lucas-koehler It seems that I cannot create branch/open MR, can you please help me it seems to be permission issue

Hi @stanisev , did you try to push the branch directly to this repository? This is not possible because you do not have write permissions. You need to fork the repository and push the branch to your fork. Then, you should be able to open a MR against this repository.

fix(angular-material): improved compatibility by removing angular fle…

430fbec

…x layout

JBBianchi added 3 commits December 14, 2023 18:15

fix(angular-test): added broader compatibility with angular

71533e8

Merge branch 'master' into angular-v15-no-flex-layout

acf87c7

fix(angular-material): fixed @angular-devkit/build-angular version

e343b9d

JBBianchi added 2 commits December 14, 2023 18:35

fix(pnpm): forced usage of angular v15

a875f74

fix(angular-material): fixed tests by removing empty component styles.

2d4599a

fix(angular): updated to Angular 16

d7ff134

fix(angular-material): fixed linting

ed3b902

sdirix force-pushed the master branch from f0f247a to 8bd14c2 Compare December 15, 2023 15:54

lucas-koehler reviewed Dec 15, 2023

View reviewed changes

sdirix force-pushed the master branch from 8bd14c2 to e9946ef Compare December 15, 2023 16:07

JBBianchi added 2 commits December 16, 2023 09:33

fix(angular-material): PR review fixes

3ebb626

- readded angularCompilerOptions in tests tsconfig - removed empty imports array in tests - applied flex to divs only in group-layout renderer - removed html comment in range renderer

Merge branch 'master' into angular-v15-no-flex-layout

6ec6c9d

lucas-koehler requested changes Dec 18, 2023

View reviewed changes

packages/angular-material/src/layouts/group-layout.renderer.ts Show resolved Hide resolved

fix(angular-material): added padding on "mat cards"

19af601

JBBianchi mentioned this pull request Dec 20, 2023

Library build of Angular packages #1546

Closed

Merge branch 'master' into angular-v15-no-flex-layout

ca30fa0

lucas-koehler requested changes Dec 22, 2023

View reviewed changes

JBBianchi added 3 commits December 23, 2023 17:20

fix(angular): dropper support for angular v15 & range control event b…

47ff208

…inding

fix(angular): dropper support for angular v15 & range control event b…

8eacb77

…inding

Merge branch 'angular-v15-no-flex-layout' of https://github.com/JBBia…

38d5960

…nchi/jsonforms into angular-v15-no-flex-layout

sdirix mentioned this pull request Jan 2, 2024

NG0300: Multiple components match node with tagname button: MatLegacyButton and MatButton error #2233

Closed

Update pnpm lock file

fe5f906

lucas-koehler reviewed Jan 2, 2024

View reviewed changes

lucas-koehler and others added 3 commits January 2, 2024 10:57

Revert "Update pnpm lock file"

d23996f

This reverts commit fe5f906.

fix(angular): removed pnpm overrides in favor of dev deps

878fbe3

Update dependencies and restrict vue via pnpm override to ~3.3.13

44244a0

Update the pnpm lock file and override vue and @vue/compiler-sfc dependencies to ~3.3.13 because vue-vanilla tests currently fail when updating to vue 3.4.x.

lucas-koehler approved these changes Jan 3, 2024

View reviewed changes

lucas-koehler merged commit c2836a5 into eclipsesource:master Jan 3, 2024
8 checks passed

lucas-koehler mentioned this pull request Jan 3, 2024

Support Angular w/ Material v16 #2138

Closed

sdirix linked an issue Jan 30, 2024 that may be closed by this pull request

Support Angular w/ Material v16 #2138

Closed

stanisev reviewed Apr 29, 2024

View reviewed changes

JBBianchi mentioned this pull request Apr 30, 2024

The delete button should be displayed only when an item is hovered and hidden #2327

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved Angular compatibility #2229

Improved Angular compatibility #2229

JBBianchi commented Dec 14, 2023 •

edited

Loading

netlify bot commented Dec 14, 2023 •

edited

Loading

JBBianchi commented Dec 14, 2023

JBBianchi commented Dec 14, 2023 •

edited

Loading

gexclaude commented Dec 15, 2023

coveralls commented Dec 15, 2023 •

edited

Loading

JBBianchi commented Dec 15, 2023 •

edited

Loading

lucas-koehler commented Dec 15, 2023

JBBianchi commented Dec 15, 2023 •

edited

Loading

JBBianchi commented Dec 15, 2023

lucas-koehler left a comment

lucas-koehler left a comment

lucas-koehler commented Dec 20, 2023

lucas-koehler left a comment

JBBianchi commented Dec 22, 2023

JBBianchi commented Dec 23, 2023

lucas-koehler left a comment

lucas-koehler Jan 2, 2024

JBBianchi Jan 2, 2024

sdirix Jan 2, 2024 •

edited

Loading

JBBianchi Jan 2, 2024

lucas-koehler left a comment

stanisev Apr 29, 2024

JBBianchi Apr 29, 2024 •

edited

Loading

stanisev Apr 29, 2024

JBBianchi Apr 29, 2024

sdirix Apr 29, 2024

stanisev Apr 29, 2024

lucas-koehler Apr 30, 2024

stanisev May 2, 2024

lucas-koehler May 3, 2024

Improved Angular compatibility #2229

Improved Angular compatibility #2229

Conversation

JBBianchi commented Dec 14, 2023 • edited Loading

netlify bot commented Dec 14, 2023 • edited Loading

✅ Deploy Preview for jsonforms-examples ready!

JBBianchi commented Dec 14, 2023

JBBianchi commented Dec 14, 2023 • edited Loading

gexclaude commented Dec 15, 2023

coveralls commented Dec 15, 2023 • edited Loading

JBBianchi commented Dec 15, 2023 • edited Loading

lucas-koehler commented Dec 15, 2023

JBBianchi commented Dec 15, 2023 • edited Loading

JBBianchi commented Dec 15, 2023

lucas-koehler left a comment

Choose a reason for hiding this comment

lucas-koehler left a comment

Choose a reason for hiding this comment

lucas-koehler commented Dec 20, 2023

lucas-koehler left a comment

Choose a reason for hiding this comment

JBBianchi commented Dec 22, 2023

JBBianchi commented Dec 23, 2023

lucas-koehler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdirix Jan 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucas-koehler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JBBianchi Apr 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JBBianchi commented Dec 14, 2023 •

edited

Loading

netlify bot commented Dec 14, 2023 •

edited

Loading

JBBianchi commented Dec 14, 2023 •

edited

Loading

coveralls commented Dec 15, 2023 •

edited

Loading

JBBianchi commented Dec 15, 2023 •

edited

Loading

JBBianchi commented Dec 15, 2023 •

edited

Loading

sdirix Jan 2, 2024 •

edited

Loading

JBBianchi Apr 29, 2024 •

edited

Loading