Skip to content

Angular 18 update #441

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

Closed
tsenguunchik opened this issue May 24, 2024 · 18 comments
Closed

Angular 18 update #441

tsenguunchik opened this issue May 24, 2024 · 18 comments

Comments

@tsenguunchik
Copy link

Angular 18 was just released today. Is it possible to update this package?

Thank you!

@Jefiozie
Copy link
Member

We just released a beta release, please test it and provide us with feedback.

angular-split 18.0.0.-beta.0

@MelanieW97
Copy link

MelanieW97 commented Jul 22, 2024

I've just tested the beta version. The property "order" seems to be missing in "as-split-area" since I get the following error message:

Can't bind to 'order' since it isn't a known property of 'as-split-area'.

Moreover, the member "IOutputData" does no longer exist:

Module 'angular-split' has no exported member 'IOutputData'.

Since the changes of version 18 have not been documented yet, I don't know what to change right there. Have the property/member names been changed?

All I did was simply upgrading to the beta version. Version 17 is working perfectly fine if being installed with "npm i --legacy-peer-deps" in order to ignore the dependencies errors, though.

@KBroichhausen
Copy link

I tested the beta version and it is not working on Firefox.
You get a simple Uncaught ReferenceError: TouchEvent is not defined
Looks like the changes in projects/angular-split/src/lib/utils.ts are not compatible with Firefox.
It breaks on this line (and will very likely break on the other usages):

if (event instanceof TouchEvent) {

That being said, it works fine on Chrome.

According to this Stackoverflow it needs an additional check: https://stackoverflow.com/questions/27313488/touchevent-not-working-in-firefox-and-other-website-browser

See https://stackblitz.com/edit/stackblitz-starters-6ytofo?file=src%2Fmain.html

@r2musings
Copy link

After upgrading, I now get this error on all of my unit tests.

FAIL src/app/shipping/components/shipment-report/shipment-report.component.spec.ts
● Test suite failed to run
ReferenceError: Cannot access 'SplitAreaComponent' before initialization
44 | import { AccountSelectionDialogComponent } from './components/account-selection-dialog/account-selection-dialog.component';
45 | import { LandingLayoutComponent } from './components/landing-layout/landing-layout.component';
> 46 | import { AngularSplitModule } from 'angular-split';
| ^
47 | import { SiteMenuComponent } from './components/site-menu/site-menu.component';
48 | import { LocalizedShortDatePipe } from './pipes/localized-short-date.pipe';
49 | import { DraggableDirective } from './directives/draggable.directive';
at Object. (node_modules/angular-split/fesm2022/angular-split.mjs:724:2005)
at Object. (src/app/shared/shared.module.ts:46:1)
at Object. (src/app/shipping/shipping.module.ts:5:1)
at Object. (src/app/shipping/components/shipment-report/shipment-report.component.spec.ts:4:1)

@Harpush
Copy link
Collaborator

Harpush commented Aug 16, 2024

@MelanieW97 There were some breaking changes indeed. They are currently not yet documented unfortunately. You can read the PR message for now: #444. In your specific use case - order input is gone as it is not needed anymore as order is based on DOM order (please do tell us if you still need it and for what reason). IOutputData has been renamed to SplitGutterInteractionEvent.

@KBroichhausen Thanks for pointing it out! You can create a PR that fixes it if you want or I will do it in the following days.

@r2musings Please provide a reproduction so we can check what the problem is.

@MelanieW97
Copy link

@MelanieW97 There were some breaking changes indeed. They are currently not yet documented unfortunately. You can read the PR message for now: #444. In your specific use case - order input is gone as it is not needed anymore as order is based on DOM order (please do tell us if you still need it and for what reason). IOutputData has been renamed to SplitGutterInteractionEvent.

Alright, thank you. I got it to work with these changes.
For us, the beta version seems to be working perfectly fine now - even without the order property. It was definitely needed in version 17, though. However, now that you use the DOM order, that seems to be enough for us to define the order.

@Xorok
Copy link

Xorok commented Aug 27, 2024

We get the same error as @r2musings after upgrading to Angular 18 when running our unit tests (ng test / npm run test) . Building the project using ng build and running it using ng serve works normally though, it's just with tests.

 FAIL  src/app/test/test-view.module.spec.ts
  ● Test suite failed to run

    ReferenceError: Cannot access 'SplitAreaComponent' before initialization

      11 | import { TreeModule } from "primeng/tree";
    > 12 | import { AngularSplitModule } from "angular-split";
      13 | import { DropdownModule } from "primeng/dropdown";
      14 | import { FormsModule } from "@angular/forms";
      15 | import { MutualModule } from "../mutual/mutual.module";

      at Object.<anonymous> (node_modules/angular-split/fesm2022/angular-split.mjs:724:2005)
      at Object.<anonymous> (src/app/test-view/test-view.module.ts:12:1)

If I remove the AngularSplitModule from the imports array in our TestViewModule, the test passes (while warning us that it now obviously doesn't recognize the components from angular-split in our HTML template, such as <as-split>, anymore). Downgrading angular-split from version 18.0.0-beta.0 to 17.2.0 also makes the error disappear and the test pass.

From what I've read, this error occurs most often when there are circular dependencies in the project. These can be shown using the package "madge":

npm install madge
npx madge --circular --extensions ts ./

After entering newer versions of the dependencies in the Stackblitz Demo's packages.json, it also shows the error:
https://stackblitz.com/edit/angular-split-demo-ggct6m

@r2musings
Copy link

Thanks @Xorok for the confirmation and the stackblitz. I had an investor demo and ended up just rolling back versions of angular-split as I couldn't risk the unit tests failing my build pipelines.

@Harpush
Copy link
Collaborator

Harpush commented Aug 27, 2024

@Xorok which test runner are you using?

@Xorok
Copy link

Xorok commented Aug 27, 2024

@Harpush The project uses Jest via @angular-builders/jest

@kainiedziela
Copy link

kainiedziela commented Sep 18, 2024

@Harpush My team is using angular-split together with <ng-template>, which would not be supported after this update, after which the order of <as-split-area> elements as written in the HTML file trumps what's actually in the DOM

Example:

<ng-container>
  <as-split>
    <!-- Left-to-right -->
    <ng-container *ngIf="leftToRight; else rightToLeft">
      <ng-container *ngTemplateOutlet="areaOneTemplate; context: { order: 0 }"></ng-container>
      <ng-container *ngTemplateOutlet="areaTwoTemplate; context: { order: 1 }"></ng-container>
    </ng-container>

    <!-- Right-to-left -->
    <ng-template #rightToLeft>
      <ng-container *ngTemplateOutlet="areaTwoTemplate; context: { order: 0 }"></ng-container>
      <ng-container *ngTemplateOutlet="areaOneTempalte; context: { order: 1 }"></ng-container>
    </ng-template>

    <ng-template #areaOneTemplate let-order="order">
      <as-split-area [order]="order">
        Area One
      </as-split-area>
    </ng-template>

    <ng-template #areaTwoTemplate let-order="order">
      <as-split-area [order]="order">
        Area Two
      </as-split-area>
    </ng-template>
  </as-split>
</ng-container>

angular-split v18.0.0 (after removing [order]) will behave as if Area One is the first in the DOM, even if an right-to-left scenario, Area Two would be first in DOM

@Harpush
Copy link
Collaborator

Harpush commented Sep 18, 2024

@Harpush My team is using angular-split together with <ng-template>, which would not be supported after this update, after which the order of <as-split-area> elements as written in the HTML file trumps what's actually in the DOM

Example:

<ng-container>
  <as-split>
    <!-- Left-to-right -->
    <ng-container *ngIf="leftToRight; else rightToLeft">
      <ng-container *ngTemplateOutlet="areaOneTemplate; context: { order: 0 }"></ng-container>
      <ng-container *ngTemplateOutlet="areaTwoTemplate; context: { order: 1 }"></ng-container>
    </ng-container>

    <!-- Right-to-left -->
    <ng-template #rightToLeft>
      <ng-container *ngTemplateOutlet="areaTwoTemplate; context: { order: 0 }"></ng-container>
      <ng-container *ngTemplateOutlet="areaOneTempalte; context: { order: 1 }"></ng-container>
    </ng-template>

    <ng-template #areaOneTemplate let-order="order">
      <as-split-area [order]="order">
        Area One
      </as-split-area>
    </ng-template>

    <ng-template #areaTwoTemplate let-order="order">
      <as-split-area [order]="order">
        Area Two
      </as-split-area>
    </ng-template>
  </as-split>
</ng-container>

angular-split v18.0.0 (after removing [order]) will behave as if Area One is the first in the DOM, even if an right-to-left scenario, Area Two would be first in DOM

I am not sure I understand... The order in the template is flipped but the order property is the same - so just don't flip it? I would be happy with a stackblitz to reproduce the problem.

@kainiedziela
Copy link

We can't "just don't flip it", a consumer being able to flip it is a part of intended functionality

I pasted my example into StackBlitz as requested: https://stackblitz.com/edit/angular-split-demo-1kyyj4

You can change leftToRight between false and true and see how the application reacts by switching the areas around

When doing this with v18.0.0-beta.0, nothing happens when changing leftToRight

@Harpush
Copy link
Collaborator

Harpush commented Sep 19, 2024

We can't "just don't flip it", a consumer being able to flip it is a part of intended functionality

I pasted my example into StackBlitz as requested: https://stackblitz.com/edit/angular-split-demo-1kyyj4

You can change leftToRight between false and true and see how the application reacts by switching the areas around

When doing this with v18.0.0-beta.0, nothing happens when changing leftToRight

That's interesting. I would bet on an angular bug here. The old way worked as order took precedence over DOM order but this is unexpected from angular. I will open an issue and see how it goes. If that is by design on the angular side the order property might need to be added back or maybe there is some workaround.
@SanderElias @Jefiozie That's really weird (how angular works here) - any idea why it happens?

@Harpush
Copy link
Collaborator

Harpush commented Sep 19, 2024

@kainiedziela Anyway I would recommend this pattern instead for your requirement:

<as-split>
  <as-split-area>
    <ng-container *ngTemplateOutlet="leftToRight ? areaOneTemplate : areaTwoTemplate"></ng-container>
  </as-split-area>
  <as-split-area>
    <ng-container *ngTemplateOutlet="leftToRight ? areaTwoTemplate : areaOneTemplate"></ng-container>
  </as-split-area>
</as-split>
<ng-template #areaOneTemplate> Area One </ng-template>
<ng-template #areaTwoTemplate> Area Two </ng-template>

Or if you really need a different as-split-area for different size or min size for example - you can do:

<as-split>
  @for (area of ['area2', 'area1']; track area) {
    @switch (area) {
      @case ('area1') {
        <as-split-area> Area One </as-split-area>
      }
      @case ('area2') {
        <as-split-area> Area Two </as-split-area>
      }
    }
  }
</as-split>

and change the areas array order as required and everything will work. This pattern also gives you the best control and performance.

@kainiedziela
Copy link

@Harpush Thanks! Worked like a charm

@Harpush
Copy link
Collaborator

Harpush commented Oct 24, 2024

For anyone interested we opened an RFC for a snap feature to replace expand/collapse.
#458

@Jefiozie
Copy link
Member

Jefiozie commented Nov 4, 2024

Version 18 is up and we are working on adding some easier transition between breaking changes. I'm closing this for now but feel free to reopen it when there are questions

@Jefiozie Jefiozie closed this as completed Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants