fix[angular-gen2]: new blocks getting added at the top while editing #3932
Conversation
🦋 Changeset detectedLatest commit: ce2e84d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
View your CI Pipeline Execution ↗ for commit ce2e84d.
☁️ Nx Cloud last updated this comment at |
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
| test.skip( | ||
| packageName === 'vue', | ||
| `Failing on the CI: TypeError: Cannot read properties of null (reading 'namespaceURI')` | ||
| ); |
There was a problem hiding this comment.
this works on local
|
@sidmohanty11 Let me know if I get this right... |
|
@shubham-builder This never gets called again and the myContent array doesn't update the nodes when blocks changes. We need to use the |
| code = code.replace( | ||
| 'ngOnChanges(changes: SimpleChanges) {', | ||
| // trigger a re-render of the view when blocks got updated and the new children content has been fully initialized | ||
| `ngAfterContentChecked() { |
There was a problem hiding this comment.
can we use our compileContext to add this instead of using a regex?
| @@ -455,4 +456,96 @@ test.describe('Visual Editing', () => { | |||
| await page.frameLocator('iframe').getByText('Bye').waitFor(); | |||
| }); | |||
| }); | |||
|
|
|||
| test.describe('New Block addition and deletion', () => { | |||
| test('should add new block below the last block', async ({ | |||
There was a problem hiding this comment.
Can we also check that adding a block in the middle from [a,b,c] to [a,b,d,c] works as expected? In case there's issues with stuff always getting pushed to the end too
| test.skip(packageName === 'nextjs-sdk-next-app'); | ||
| test.skip( | ||
| packageName === 'vue', | ||
| `Failing on the CI: TypeError: Cannot read properties of null (reading 'namespaceURI')` |
There was a problem hiding this comment.
I've seen this intermittently happen in multiple tests...no clue what it's about yet
| code = code.replace(/^\s*\/\/\s*@ts-expect-error.*$/gm, ''); | ||
| code = code.replace( | ||
| 'ngOnChanges(changes: SimpleChanges) {', | ||
| // trigger a re-render of the view when blocks got updated and the new children content has been fully initialized |
There was a problem hiding this comment.
Could we document here in more detail? A brief summary of why it's needed, essentially a tl;dr of your looms. You'll know how to phrase this in the most accurate way, but something along the lines of:
Since the angular SDK manually handles the creation of the dynamic blocks and attaching them as children of
BlocksWrapperin the DOM, it must also manually handle their re-renders on content change in the visual editor.ngAfterContentCheckedruns after children were checked for changes, which is the earlier point we can safely append new blocks, otherwise they are appended outside the existing array of Blocks, therefore pushed to the top of the list.
Description
https://builder-io.atlassian.net/browse/ENG-7718
Screenshot
If relevant, add a screenshot or two of the changes you made.