Skip to content

Feat/headless function#290

Open
lagartoverde wants to merge 5 commits into
masterfrom
feat/headless-function
Open

Feat/headless function#290
lagartoverde wants to merge 5 commits into
masterfrom
feat/headless-function

Conversation

@lagartoverde
Copy link
Copy Markdown
Contributor

Overview

Make the user able to run headless functions, this combines with the locked placeholder replace function that needs to be run headlessly by CEVI

connected issues and PRs:

lblod/ember-rdfa-editor-lblod-plugins#656

Setup

Check that you can run headless functions in the editor, in order to make the testing easier I built a new page where clicking a button lets you change the besluit type of a besluit headlessly

How to test/reproduce

Go to the new page: headless test, insert a besluit in the besluit textarea, insert a besluit uri in the besluit type input, click on the button notice your besluit type gets changed for the type you inserted

Challenges/uncertainties

Once this PR is approved need to document everything, please if you think something about the API could be improved let me know

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
  • no new deprecations

Copy link
Copy Markdown
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.

Seems to work well. One thing that feels missing is that there's no feedback on failure. I get that the processing code is taken from GN and that this is missing there, but it would most likely be good to have there too (and maybe that function should move to the editor or plugins repo?

@abeforgit
Copy link
Copy Markdown
Member

One thing to watch out as well is that now you're importing from the plugin repo
In a typical project that uses embeddable, the only dependency they have is the embeddable itself.
We'll have to re-expose the functions we want them to use here as well.

@lagartoverde could I ask you to do the following as well, in a separate pr:

  • update the editor and plugin deps to the version you need for the locked placeholder feature
  • set up a test page with some locked placeholders, and a button to replace them

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