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

Feature/gn 3152 create a debug component for the rdfa editor #206

Conversation

benjay10
Copy link
Contributor

You should be able to use a debug component for the editor by using <Rdfa::RdfaEditorWithDebug .../> instead of the normal <Rdfa::RdfaEditor .../> component with the same arguments. For example:

<Rdfa::RdfaEditorWithDebug
  @rdfaEditorInit={{this.rdfaEditorInit}}
  @editorOptions={{hash
    showToggleRdfaAnnotations="true"
    showInsertButton=null
    showRdfa="true"
    showRdfaHighlight="true"
    showRdfaHover="true"
    showPaper="true"
    showSidebar="true"
    showToolbarBottom=null
  }}
  @toolbarOptions={{hash
    showTextStyleButtons="true"
    showListButtons="true"
    showIndentButtons="true"
  }}
/>

@benjay10 benjay10 added the enhancement New feature or request label Jan 19, 2022
@benjay10 benjay10 marked this pull request as ready for review January 20, 2022 14:07
@nvdk nvdk requested a review from abeforgit January 24, 2022 08:59
Copy link
Member

@abeforgit abeforgit left a comment

Choose a reason for hiding this comment

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

Looks good overall. Main thing is the branch, we tend to branch our feature branches off of development and make PRs against that.
I can resolve the conflicts for you if you're not comfortable doing that

tests/dummy/app/router.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@abeforgit abeforgit changed the base branch from master to development January 24, 2022 21:55
@abeforgit
Copy link
Member

abeforgit commented Jan 24, 2022

Looks good overall. Main thing is the branch, we tend to branch our feature branches off of development and make PRs against that. I can resolve the conflicts for you if you're not comfortable doing that

about this, notice ive changed the base branch of the PR already. Here's how to proceed:

  • fix the other review comments first
  • rebase this branch on top of development (rebasing can be annoying, we can go through this together*)
  • git push --force-with-lease (always use force-with-lease, never --force, its basically a safer version of force preventing you from overriding other people's work)

* EDIT: just tried it, the conflicts are minor, so it can be good practice

“[BenJay10]” added 6 commits January 25, 2022 01:07
All code and testdata was copied from the dummy app to the addon. All
things seem to work, apart from arguments to the debug component.
Also rebase to development.
The normal arguments like rdfaEditorInit callback, toolbarOptions, and
editorOptions are now supported (passed through) on the debug component.
These files made the drone builds fail
Component arguments didn't have the correct type yet. Added a TypeScript
interface and added the type generics to the inherited Component class.
Removed the debug route and replaced the original index route with the
new debug component.
Also rebase to development.
@benjay10 benjay10 force-pushed the feature/GN-3152-create-a-debug-component-for-the-rdfa-editor branch from fd62693 to aa988e5 Compare January 25, 2022 00:11
@abeforgit abeforgit merged commit 787ae37 into development Jan 25, 2022
@abeforgit abeforgit deleted the feature/GN-3152-create-a-debug-component-for-the-rdfa-editor branch January 25, 2022 10:03
@benjay10 benjay10 restored the feature/GN-3152-create-a-debug-component-for-the-rdfa-editor branch January 25, 2022 23:18
@benjay10 benjay10 deleted the feature/GN-3152-create-a-debug-component-for-the-rdfa-editor branch January 25, 2022 23:41
@benjay10
Copy link
Contributor Author

I tried reopening the branch to see if I could add more fixes to this thing, but I then forgot to switch to that branch and pushed to development instead. I hope nothing breaks because of the stream-browserify dependency.

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.

2 participants