feat: modernize angular split #444

Harpush · 2024-05-27T07:30:30Z

Features

Using css grid instead of flex allows for fraction usage in calculations and percent based on areas without gutters. In addition it will allow later to add fraction unit type for easy 1/3 size splitting and maybe will allow mixing percent and pixels sizes too.
Fully signal based (data, inputs, outputs, effects)
Using the new control flow
No more order problems when hiding and showing areas (that's why the input order was created before)
Easier algorithm for dragging which also solves some bugs
Everything is standalone
Global defaults provider now use the new provideX pattern and adds type safety
Aria values are now updating live while dragging
Delta pixel now accurately delay drag start by the delta specified unless going out of gutter
Interface names are finally without the initial I which isn't recommended for typescript
No more imperative style update and validation thanks to signals
Gutter now stays (with zero size) while area is invisible which allows for gutter transitions as well

Breaking(ish) changes

Aria values are now updating live while dragging
Delta pixel now accurately delay drag start by the delta specified unless going out of gutter
Only left mouse click will start dragging compared to right click too previously
CSS grid and not flex is used
When passing wrong data previously it would silently "fix" itself. Now in dev mode it will also write console warnings as this is unexpected usage
As we are now using CSS grid - the minimum browser requirement is higher especially for transitions.
Four fast clicks will only emit one double click event and not two. Same for more clicks.
Invisible area gutter is still in the DOM but with zero size

Breaking changes

Order input is now gone as order is taken strictly from DOM order. Can be added back if required but seems like larger API surface with low benefit
Split area is now a component and not a directive - that means no more attribute selector
Interface names changed
ANGULAR_SPLIT_DEFAULT_OPTIONS is no longer exported - use the new provideAngularSplitOptions
Due to signal inputs, altering the inputs using component TS reference is no longer possible (see access from class demo)

Tests

Cypress has been updated as electron didn't support CSS grid transitions
Gutter step was somehow rounding 15px to 10px and not to 20px - tests changed but I can investigate further why it happened
I added rounding to sub pixel (1 number after dot) in all tests as sizes vary a bit from old way and between live/ci test runs

Missing features (require discussion)

expand/collapse feature + demo (currently broken) - I really don't understand this feature... it resizes the area and disable the gutter (making it smaller too). I don't really see how this is a split feature.
getVisibleAreaSizes - easy to implement but I think make sense only when set exists
setVisibleAreaSizes - Allows altering the size from outside without the component binding. I am not sure this is desirable. The only benefit I can see is less change detection on parent component when using drag progress (see sync split demo)

Out of scope

Tests pixels fixing and adding new tests
Demos transition to signals, standalone, etc...

I might have forgotten some features or breaking changes....

Closes #135
Closes #426
Closes #378
Closes #439
Closes #433
Closes #267

bastienmoulia · 2024-06-03T08:17:36Z

Is it possible to add more scss variables with default value in order to customize:

the background-color of the bar (#eeeeee => $as-background)
the images in the center of the bar (url('data:image/png;base64,...') => $as-horizontal-icon)

Or to use css variable, it can also work :)

Harpush · 2024-06-03T21:14:51Z

Is it possible to add more scss variables with default value in order to customize:

the background-color of the bar (#eeeeee => $as-background)

the images in the center of the bar (url('data:image/png;base64,...') => $as-horizontal-icon)

Or to use css variable, it can also work :)

That's unrelated to this PR and already got merged in #367

Harpush · 2024-06-03T21:16:40Z

@SanderElias @Jefiozie the merge conflicts are only due to CSS variables PR.
You can still go over this one even though there are merge conflicts which I will solve only next week unfortunately.

godind · 2024-06-07T01:34:16Z

Anxious to see this new version. I was in the process of updating my dashboard app to try and include cdk drag & drop. I'll not waist time and wait a bit so I'll be available as a beta tester if needed.

Thanks for all the great work you are doing!!!

mrentmeister-tt · 2024-06-11T14:32:21Z

I currently use [order] on split areas that are included/excluded from the dom with an @if/ngIf. Without setting the order, the areas included by the @if/ngIf always get rendered after the static area(s). Would this PR break this functionality?

Example:

<as-split
  direction="horizontal"
  unit="percent"
  [gutterSize]="8"
>
  @if (currentHelp()) {
    <as-split-area class="tt-help-split-area" [order]="0">
      ...
    </as-split-area>
  }
  <as-split-area size="*" [order]="1"><ng-content></ng-content></as-split-area>
</as-split>

Harpush · 2024-06-11T14:38:30Z

I currently use [order] on split areas that are included/excluded from the dom with an @if/ngIf. Without setting the order, the areas included by the @if/ngIf always get rendered after the static area(s). Would this PR break this functionality?

Example:
<as-split
  direction="horizontal"
  unit="percent"
  [gutterSize]="8"
>
  @if (currentHelp()) {
    <as-split-area class="tt-help-split-area" [order]="0">
      ...
    </as-split-area>
  }
  <as-split-area size="*" [order]="1"><ng-content></ng-content></as-split-area>
</as-split>

With this PR the order is based on DOM order. That means if the ngIf adds the area before the static ones it will work as expected.

will avoid multiple warnings in console and the main/max size won't suddenly start working while dragging

…cts sizes" This reverts commit 5fc3aa9.

SanderElias · 2024-06-19T14:33:30Z

@Jefiozie Would you have time to review this?
This would be a breaking version, but it takes care of quite some things we have on our list. I did review it lightly, but currently don't have time to go in deeper.
I'm ok with the changes, have discussed most of my concerns with @Harpush , and I think he has addressed those.

Jefiozie

I left some questions but nothing serious. My main concern is that I would like to have a deprecation notice before we remove the old types OR we need to have a schematic in place to migrate everything.

Im not against either one but backwars compatibily is something i would appriciate.

Jefiozie · 2024-06-20T17:52:39Z

cypress/e2e/7.click.cy.js

@@ -35,32 +35,32 @@ context('Gutter click example page tests', () => {

 it('Click gutters to switch area sizes between 0 and X', () => {
 cy.get('.as-split-gutter').eq(0).click()
- cy.wait(1500)
+ cy.wait(2000)


WHy is this change needed for cypress?

I can't seem to figure out how it worked before. The transition is 1.5s and the delay is 1.5s. When running live it works well but fails in CI run.
At 1500 it fails on 9 pixels instead of 0
At 1600 it fails on 5 pixels instead of 0
At 2000 it passes

Not sure what to do about it...

Jefiozie · 2024-06-20T17:56:43Z

package.json

@@ -62,10 +58,14 @@
 "@types/node": "20.5.4",
 "@typescript-eslint/eslint-plugin": "6.19.0",
 "@typescript-eslint/parser": "6.19.0",
- "cypress": "12.17.4",
+ "angular-cli-ghpages": "^2.0.0-beta.2",


Can we use a none beta version or is there a issue with this version?

That's a great question. I don't know why this is beta. If you look at line 43 before my changes it was beta to begin with. The cypress update just moved those dependencies around (ABC order)

projects/angular-split/src/lib/angular-split-config.token.ts

Jefiozie · 2024-06-20T18:02:23Z

projects/angular-split/src/lib/models.ts

Could we make the old types depracted and not delete them straight away? It would be nice if we could make it so that people can use both for a period of time.
What do you think of this?

Harpush · 2024-06-20T18:47:54Z

I left some questions but nothing serious. My main concern is that I would like to have a deprecation notice before we remove the old types OR we need to have a schematic in place to migrate everything.

Im not against either one but backwars compatibily is something i would appriciate.

@Jefiozie Concerning deprecation and migrations - let's review what needs discussion:

expand/collapse feature - Should I add it back as it was but deprecated? Should we replace it with some kind of snap feature like vscode? Maybe delete altogether (breaking)?
getVisibleAreaSizes/setVisibleAreaSizes - What do we want to do about those? Deprecate maybe? Delete (breaking)?
Order input - Not needed anymore so we can either migrate or deprecate. I don't really think we want to keep supporting it. The problem is when someone used it not in the same order as the DOM.
split-area used as directive can be migrated with schematics. Deprecation isn't possible here.
ANGULAR_SPLIT_DEFAULT_OPTIONS can be deprecated or migrated.
component TS reference input changing - nothing we can really do about it... that's just not supported anymore with signal inputs.
Interface names - either migrate or export old names as type aliases of new names with deprecation.

If we go the schematics direction - we need to create the required infra as there is no schematics collection project in this repo.

ranitg · 2024-06-27T12:25:05Z

When will this be available?

Harpush · 2024-06-27T17:36:46Z

@Jefiozie I wrote some considerations above - can you share your opinion on them?

ranitg · 2024-06-27T18:55:00Z

@Harpush I actually am using the collapse/expand for snapping like in the vscode. Can you please maybe add a snap option? I had to sortof implement it with the collapse/expand feature. Don't remove it please.

Harpush · 2024-06-27T19:26:01Z

@Harpush I actually am using the collapse/expand for snapping like in the vscode. Can you please maybe add a snap option? I had to sortof implement it with the collapse/expand feature. Don't remove it please.

Can you please elaborate how you would expect the snap to work? Visible vs @if and size restoration.
The main reason why collapse seemed like a weird feature to me is the ability to collapse to non zero values. A snap feature is indeed expected from a split component imo.

godind · 2024-07-09T12:03:17Z

Is this great upgrade planned to be available soon? Can't wait to try it out!

Jefiozie · 2024-07-10T04:24:04Z

@Harpush we've decided to leave it as is and merge it. We will do a beta release of the package soon.

Harpush · 2024-07-10T05:02:26Z

@Jefiozie I wrote some considerations above - can you share your opinion on them?

In that case how about an RFC on a snap feature to replace expand/collapse?

Jefiozie · 2024-07-10T05:19:43Z

That is a good idea, would you be interested in writing the first draft/ iteration?

Harpush · 2024-07-10T05:38:03Z

That is a good idea, would you be interested in writing the first draft/ iteration?

Sure I have something in mind that should also answer most of the collapse/expand intended usage hopefully.
I just need guidance as to where to send you the draft or should I publish it as a GitHub issue directly?

Jefiozie · 2024-07-14T19:05:03Z

Hi everyone,

We just released a beta version of angular-split, you can find it angular-split 18.0.0-beta.0

feat: modernize angular split

f36da15

Harpush added the breaking changes label May 27, 2024

Harpush requested review from SanderElias and Jefiozie May 27, 2024 07:30

Harpush added 12 commits June 12, 2024 12:49

fix: typo, left click check not needed

45db74b

fix: force gutter events cancellation when inputs change affects sizes

5fc3aa9

fix: remove not needed take inside takeUntil

b06ea6d

fix: pack inputs together

15f2f11

fix: add min/max classes only when area visible

28002a2

fix: normalize min/max size based on outer size

cedf889

will avoid multiple warnings in console and the main/max size won't suddenly start working while dragging

fix: typos

2f30982

fix: delta check with custom gutter

bff7d73

feat: merge and apply css variables

f55e174

Revert "fix: force gutter events cancellation when inputs change affe…

fe906b4

…cts sizes" This reverts commit 5fc3aa9.

fix: minor fixes (typos, unused code)

8d6d10d

feat: support hierarchical inheritance in options

4ee3e27

goetzrobin mentioned this pull request Jun 20, 2024

RFC: Resizable goetzrobin/spartan#120

Open

2 tasks

Jefiozie reviewed Jun 20, 2024

View reviewed changes

fix: typo in config token comment

19502de

Jefiozie merged commit d8ce04a into angular-split:main Jul 10, 2024
5 checks passed

github-actions bot mentioned this pull request Jun 3, 2024

chore(main): release 15.1.0 #437

Open

Harpush deleted the modernize-angular-split-refactor branch July 10, 2024 05:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: modernize angular split #444

feat: modernize angular split #444

Harpush commented May 27, 2024 •

edited

Loading

bastienmoulia commented Jun 3, 2024

Harpush commented Jun 3, 2024

Harpush commented Jun 3, 2024

godind commented Jun 7, 2024 •

edited

Loading

mrentmeister-tt commented Jun 11, 2024 •

edited

Loading

Harpush commented Jun 11, 2024

SanderElias commented Jun 19, 2024

Jefiozie left a comment

Jefiozie Jun 20, 2024

Harpush Jun 20, 2024

Jefiozie Jun 20, 2024

Harpush Jun 20, 2024

Jefiozie Jun 20, 2024

Harpush commented Jun 20, 2024 •

edited

Loading

ranitg commented Jun 27, 2024

Harpush commented Jun 27, 2024

ranitg commented Jun 27, 2024

Harpush commented Jun 27, 2024

godind commented Jul 9, 2024

Jefiozie commented Jul 10, 2024

Harpush commented Jul 10, 2024

Jefiozie commented Jul 10, 2024

Harpush commented Jul 10, 2024

Jefiozie commented Jul 14, 2024

feat: modernize angular split #444

feat: modernize angular split #444

Conversation

Harpush commented May 27, 2024 • edited Loading

Features

Breaking(ish) changes

Breaking changes

Tests

Missing features (require discussion)

Out of scope

bastienmoulia commented Jun 3, 2024

Harpush commented Jun 3, 2024

Harpush commented Jun 3, 2024

godind commented Jun 7, 2024 • edited Loading

mrentmeister-tt commented Jun 11, 2024 • edited Loading

Harpush commented Jun 11, 2024

SanderElias commented Jun 19, 2024

Jefiozie left a comment

Choose a reason for hiding this comment

Jefiozie Jun 20, 2024

Choose a reason for hiding this comment

Harpush Jun 20, 2024

Choose a reason for hiding this comment

Jefiozie Jun 20, 2024

Choose a reason for hiding this comment

Harpush Jun 20, 2024

Choose a reason for hiding this comment

Jefiozie Jun 20, 2024

Choose a reason for hiding this comment

Harpush commented Jun 20, 2024 • edited Loading

ranitg commented Jun 27, 2024

Harpush commented Jun 27, 2024

ranitg commented Jun 27, 2024

Harpush commented Jun 27, 2024

godind commented Jul 9, 2024

Jefiozie commented Jul 10, 2024

Harpush commented Jul 10, 2024

Jefiozie commented Jul 10, 2024

Harpush commented Jul 10, 2024

Jefiozie commented Jul 14, 2024

Harpush commented May 27, 2024 •

edited

Loading

godind commented Jun 7, 2024 •

edited

Loading

mrentmeister-tt commented Jun 11, 2024 •

edited

Loading

Harpush commented Jun 20, 2024 •

edited

Loading