Bugfix/autofill variable bug #792
Conversation
|
Just a small comment already, will do a full review after: putting the creation of a prosemirror schema in a dynamic getter can cause issues. I'll try to dig up the original PR where the change was introduced to move away from dynamic getters for schemas. Edit, found it: https://binnenland.atlassian.net/browse/GN-4206 You can reproduce the issue when trying to paste a link. |
|
gonna also leave this one out of todays hotfix, not so urgent and would be good to test |
app/controllers/agendapoints/edit.js
Outdated
| get citationPlugin() { | ||
| return this.agendapointEditor.citationPlugin; |
There was a problem hiding this comment.
Maybe it's better to also move this to the getSchemaAndPlugins hook.
There was a problem hiding this comment.
This needs to be like this because the card makes reference to the plugin
There was a problem hiding this comment.
Yes I understand why it's needed, but making it a dynamic getter will break the autocomplete of the citation-plugin.
As the citationPlugin you pass to the card is not the same one you pass to prosemirror.
app/controllers/agendapoints/edit.js
Outdated
| const { schema, plugins } = | ||
| this.agendapointEditor.getSchemaAndPlugins(false); | ||
| this.schema = schema; | ||
| const citationPlugin = this.agendapointEditor.citationPlugin; |
There was a problem hiding this comment.
so now this means that anywhere we use this service, we just have to remember to treat the citationPlugin property in this special way?
this doesn't seem like a good solution...
something is off here, we're making a fundamentally simple thing complicated
ember is not being very helpful in all this, but I do have some ideas, I'll have a look
elpoelma
force-pushed
the
bugfix/autofill-variable-bug
branch
from
48cc4cf to
efed22f
Compare
elpoelma
changed the base branch from
master
to
internal/update-editor-packages
elpoelma
changed the base branch from
internal/update-editor-packages
to
master
elpoelma
force-pushed
the
bugfix/autofill-variable-bug
branch
from
efed22f to
27e2ebc
Compare
elpoelma
force-pushed
the
bugfix/autofill-variable-bug
branch
from
27e2ebc to
5294b67
Compare
Overview
The autofill variable bug described in GN-5210 was caused by the agendapoints.edit controller setting the plugins before the route was loaded and the current session object was filled. I reworked that interaction so nothing is called until everything has loaded correctly
connected issues and PRs:
GN-5210
Setup
None
How to test/reproduce
You can try reproducing the bug, basically if you create an IV, go to the first agendapoint and in the besluit snippet list you selected one of the options with the autofill variable the first time it should work, but if you refresh and do the same again it didn't work.
Now it always work, as the autofill plugin is not set up until everything has been loaded
Challenges/uncertainties
Not enough knowledge of the headless editor so I think I did everything correctly so that also works, but please Elena verify :)
Checks PR readiness