FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

refactor: drop signal writes effect by Harpush · Pull Request #455 · angular-split/angular-split · GitHub

refactor: drop signal writes effect - #455

Merged
Jefiozie merged 1 commit into
angular-split:mainfrom
Harpush:no-signal-writes-effect
Jan 19, 2025
Merged

refactor: drop signal writes effect#455
Jefiozie merged 1 commit into
angular-split:mainfrom
Harpush:no-signal-writes-effect

Conversation

Harpush commented Sep 17, 2024
edited
Loading

Copy link
Copy Markdown
Collaborator

Drop the validation effect that also updates internal sizes (with allow signal writes) and use computed instead as per the official angular recommendation.

The signal flow is now:

  1. SplitArea has visible and size
  2. Split creates _visibleAreas based on SplitArea visible
  3. Split creates _alignedVisibleAreasSizes based on _visibleAreas and SplitArea size with validation alignments
  4. SplitArea _internalSize is based on _alignedVisibleAreasSizes
  5. _internalSize is the actual size anything else should use (except validations and normalizations)

Copy link
Copy Markdown
Contributor

hi @Harpush is this ready for review?

Copy link
Copy Markdown
Contributor

Hi @Harpush could you resolve the conflict? I would like to add this to the beta release of 19

Harpush force-pushed the no-signal-writes-effect branch from 8396f22 to 23e36be Compare January 18, 2025 12:26
Harpush force-pushed the no-signal-writes-effect branch from 23e36be to afd4840 Compare January 18, 2025 12:49
Harpush marked this pull request as ready for review January 18, 2025 12:49
Harpush requested a review from SanderElias January 18, 2025 12:49

Harpush commented Jan 18, 2025

Copy link
Copy Markdown
Collaborator Author

@Jefiozie @SanderElias Resolved conflicts, changed a bit of the code and now it is ready for review

Jefiozie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGMT, will update v19 branch with these changes

Jefiozie merged commit f2219e8 into angular-split:main Jan 19, 2025
Harpush deleted the no-signal-writes-effect branch January 19, 2025 18:56
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

3 participants


Back | FazBrowse Home | New Git URL