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

Update editor to rdfa-backwards-compatibility build #395

Merged
merged 19 commits into from
Apr 4, 2024

Conversation

elpoelma
Copy link
Collaborator

@elpoelma elpoelma commented Mar 11, 2024

Overview

This PR updates the editor to the rdfa-backwards-compatible build (lblod/ember-rdfa-editor#1155)
This build includes some API changes:

  • rdfaAttrSpec is now a function
  • usage of the ...WithConfig configurable nodespecs

This PR also includes some changes to the citation plugin to support the rdfaAware system.

connected issues and PRs:

lblod/ember-rdfa-editor#1155

How to test/reproduce

  • pnpm start
  • Visit the different dummy pages, all node-specs should be in rdfaAware mode and the rdfa-features should work as expected

Challenges/uncertainties

It does not seem very useful to also convert all the nodespecs provided by this package to be backwards compatible.

Checks PR readiness

  • UI: works on smaller screen sizes
  • UI: feedback for any loading/error states
  • Check if dummy app is correctly updated
  • Check cancel/go-back flows
  • changelog
  • npm lint
  • no new deprecations

@elpoelma elpoelma added the enhancement New feature or request label Mar 11, 2024
@elpoelma elpoelma requested review from piemonkey and abeforgit March 11, 2024 12:45
tests/utils/editor.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@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.

When trying to insert anything from the sidebar (in the index route), I get an error:

Uncaught TypeError: can't access property "indexOf", tagName is undefined

@elpoelma
Copy link
Collaborator Author

When trying to insert anything from the sidebar (in the index route), I get an error:

Uncaught TypeError: can't access property "indexOf", tagName is undefined

Interesting, will check if I can reproduce that.

@elpoelma
Copy link
Collaborator Author

When trying to insert anything from the sidebar (in the index route), I get an error:

Uncaught TypeError: can't access property "indexOf", tagName is undefined

Does it occur with a specific action?

@piemonkey
Copy link
Collaborator

When trying to insert anything from the sidebar (in the index route), I get an error:

Uncaught TypeError: can't access property "indexOf", tagName is undefined

Does it occur with a specific action?

It happens in the insertStructure command when trying to use any of the article structures in the insert menu in the sidebar. Actually, it's perhaps just insert article but once it has happened once it happens with anything from then on.

@elpoelma
Copy link
Collaborator Author

When trying to insert anything from the sidebar (in the index route), I get an error:

Uncaught TypeError: can't access property "indexOf", tagName is undefined

Does it occur with a specific action?

It happens in the insertStructure command when trying to use any of the article structures in the insert menu in the sidebar. Actually, it's perhaps just insert article but once it has happened once it happens with anything from then on.

That specific issue should be fixed now, it was an issue in the toDOM function of the besluit_article_header nodespec.
I did notice that the current implementation of a decision article is far from usable, so we should probably fix it before releasing the plugins/going to embeddable.

@abeforgit abeforgit merged commit 3cc0097 into feat/rdfa-editing-ui Apr 4, 2024
0 of 3 checks passed
@abeforgit abeforgit deleted the wip/update-editor branch April 4, 2024 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants