Skip to content
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

GN-4529: ensure editor is only initialized after prefix attribute of wrapping div has been set #591

Merged
merged 2 commits into from
Oct 12, 2023

Conversation

elpoelma
Copy link
Contributor

@elpoelma elpoelma commented Oct 6, 2023

Overview

This PR ensures that the editor component is only initialized after the prefix attribute of it's wrapping container has been set.

This PR:

  • introduces a setUpPrefixAttr modifier on the wrapping container which sets the prefix attribute. It also controls the ready property which indicates when the editor may be initialized.
  • After the prefix attribute has been set, the ready property is set to true
  • When the element linked to the setUpPrefixAttr modifier is destroyed, the ready property is set to false (as the prefix attribute no longer exists)
    => this also means that the setUpPrefixAttr modifier is not generic and is only meant to be used in combination with the wrapping container.

This PR should solve the issue with the datastore calculation after having saved a document. (When the datastore is calculated, the prefix attribute should be set)

connected issues and PRs:

Should solve GN-4529

How to test/reproduce

  • Create an empty agendapoint
  • Notice that you have the option to insert a template
  • Save the document
  • The options to insert templates should still be there.

Challenges/uncertainties

Checks PR readiness

  • UI: works on smaller screen sizes
  • UI: feedback for any loading/error states
  • Check cancel/go-back flows
  • Check database state correct when deleting/updating (especially regarding relationships)
  • changelog
  • npm lint

Copy link
Contributor

@piemonkey piemonkey left a comment

Choose a reason for hiding this comment

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

Huh, I don't get why this wasn't working with did-insert as I thought EditorContainer should only be initialised once this.ready is set to true?

Anyway, looks good.

@elpoelma
Copy link
Contributor Author

elpoelma commented Oct 6, 2023

Huh, I don't get why this wasn't working with did-insert as I thought EditorContainer should only be initialised once this.ready is set to true?

Anyway, looks good.

Yes, but the this.ready attribute was never reset to false when saving.

@elpoelma elpoelma merged commit d49f154 into master Oct 12, 2023
@elpoelma elpoelma deleted the fix/ensure-editor-init-after-prefix-init branch October 12, 2023 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants