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

DSP-682 Integrate Ckeditor into Property View #199

Merged
merged 17 commits into from
Oct 6, 2020

Conversation

mdelez
Copy link
Contributor

@mdelez mdelez commented Oct 1, 2020

resolves DSP-682

ValueTypeService.isReadOnly has a a second required param now: isReadOnly(valueTypeOrClass: string, value: ReadValue): boolean

@mdelez mdelez added the enhancement New feature or request label Oct 1, 2020
@mdelez mdelez self-assigned this Oct 1, 2020
@mdelez
Copy link
Contributor Author

mdelez commented Oct 1, 2020

@tobiasschweizer here is what I have so far, including broken tests 😄

@tobiasschweizer
Copy link
Contributor

tobiasschweizer commented Oct 2, 2020

TODOs:

  • if the XML text value uses the standard mapping, treat it as if it were HTML in read-mode (links should clickable, formatting is rendered)

  • if the XML text value uses the standard mapping, it can be edited by the user, the necessary permissions provided

  • for all other XML text values, show the XML source in read-mode

  • whitespace issues should be dealt with in the XML component directly, not in the base class's standard validator. It would possible to override the standardValueComparisonFunc in the XML text value component as we do already for complex values like intervals etc.

  • since we only support editing of XML text values using the standard mapping, the XML conversion needed for compatibility reasons should be moved from the config object into the component itself, but be centralised there, so that the conversion works both ways. Before passing the XML to CKEditor, the conversion should be applied because otherwise the validation does not work properly (comparison of init value).

  • callback for internal url condition should check if url is null
    return !!url && url.startsWith(this.resourceBasePath);

  • css for read-mode should be adapted to remove the margin around paragraphs

  • implement support for adding a new XML value

@tobiasschweizer
Copy link
Contributor

tobiasschweizer commented Oct 2, 2020

Conversion fun:

"xmlTransform": {
    "http://rdfh.ch/standoff/mappings/StandardMapping": {
      "<hr>": "<hr/>",
      "</hr>": "",
      "<s>": "<strike>",
      "</s>": "</strike>",
      "<i>": "<em>",
      "</i>": "</em>",
      "<figure class=\"table\">": "",
      "</figure>": ""
    }
  }

Maybe we could make the CKEditor instance return the internally converted markup, so we could use it as the initial value.

Maybe related: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/deep-dive/conversion/conversion-preserving-custom-content.html

@tobiasschweizer
Copy link
Contributor

@mdelez Could you have a look at 2627050? I had to adapt some expectations in the tests.

@tobiasschweizer
Copy link
Contributor

tobiasschweizer commented Oct 5, 2020

@mdelez I expanded the read-only check for XML values based on the mapping. However, this required a change in the value-type service's isReadOnly method. Is that ok? Since the service is provided in root, this is a breaking change.

done in 598abb5

@mdelez
Copy link
Contributor Author

mdelez commented Oct 5, 2020

@mdelez I expanded the read-only check for XML values based on the mapping. However, this required a change in the value-type service's isReadOnly method. Is that ok? Since the service is provided in root, this is a breaking change.

done in 598abb5

Seems fine to me

@tobiasschweizer tobiasschweizer added the breaking Breaking change label Oct 5, 2020
@tobiasschweizer
Copy link
Contributor

@mdelez awesome, I could add a new text!!!

@mdelez mdelez merged commit 2835864 into master Oct 6, 2020
@mdelez mdelez deleted the wip/dsp-682-integrate-ckeditor branch October 6, 2020 14:09
@kilchenmann kilchenmann changed the title Integrate Ckeditor into Property View DSP-682 Integrate Ckeditor into Property View Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants