Simplify replacement variable editor logic to set id and ref.

afercia commented 5 years ago

Summary

This PR can be summarized in the following changelog entry:

removes the id prop from the draft.js Editor as it's not an allowed prop and it doesn't do anything
removes a try/catch statement from the setEditorRef() function: this statement tried to set an id to the draft.js editable field using DOM methods:
- not sure why we should try to set the id here, as it's already set by setEditorFieldId() when the component mounts. The whole statement seems safe to remove to me, unless I'm missing something.
- worth noting when a component gets unmounted, React runs something called safelyDetachRef which calls the ref callback and the DOM method used there failed, triggering the console.trace() message
simplifies setEditorFieldId(): seems to me the draft.js editor ref already has a editor.editor property which is the field DOM element so seems to me there's no need to use DOM methods to get the parent and the child again with editorContainer.querySelector( "div[contenteditable]" )

in the standalone app > Snippet preview tab
click "Edit snippet" to open and close the editor
verify there are no JS errors or console output (note: I get an unrelated Error: Domainyoast-componentswas not found.
verify the title and description field have correct IDs:
- snippet-editor-field-title
- snippet-editor-field-description
in the snippet preview, click on the title and the description, test it when the editor is closed and also when it's already open and verify the related field get focused

abotteram commented 5 years ago

CR 👍

Dieterrr commented 5 years ago

Acceptance done 👍