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: Serious ckeditor inlineMode isInline #3553

Draft
wants to merge 4 commits into
base: 8.4
Choose a base branch
from

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Jun 29, 2023

resolves #3545

What I did

How I did it

How to verify it

todo:

  • pasting newlines should insert brs

@mhsdesign mhsdesign requested a review from grebaldi June 29, 2023 20:29
@github-actions github-actions bot added Feature Label to mark the change as feature 8.3 labels Jun 29, 2023
@mhsdesign mhsdesign force-pushed the feature/seriousCkInlineMode branch from 429f2e4 to 1097108 Compare June 29, 2023 20:32
@grebaldi
Copy link
Contributor

E2E tests:
image

Didn't know it was supposed to 😄

@@ -21,7 +21,7 @@
inlineEditable: true
inline:
editorOptions:
autoparagraph: false
isInline: true
Copy link
Member Author

Choose a reason for hiding this comment

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

  • inlineMode?
  • inlineContentMode?
  • block = false

or decide in fusion?

Neos.Fusion:Editable {
   block = false
}

Copy link
Member Author

Choose a reason for hiding this comment

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

We decided to use the the same naming as in fusion: block false.

And its allowed to either specify it in the nodeType or via the rendering.

Following logic would apply:

    const isInlineMode = editorOptions?.block === false ||
         propertyDomNode.tagName === 'SPAN' ||
         propertyDomNode.tagName === 'H1' ||
         propertyDomNode.tagName === 'H2' ||
         propertyDomNode.tagName === 'H3' ||
         propertyDomNode.tagName === 'H4' ||
         propertyDomNode.tagName === 'H5' ||
         propertyDomNode.tagName === 'H6' ||
         propertyDomNode.tagName === 'P';

happy cases:

editorOptions: { } | { block: true } + Neos.Fusion:Editable { } | { block: true } -> <div>{isInlineMode: false}</div>
editorOptions: { } + Neos.Fusion:Editable { block: false } -> <span>{isInlineMode: true}</span>
editorOptions: { block: false } + Neos.Fusion:Editable { block: false } -> <span>{isInlineMode: true}</span>

but this would also lead to cases like this:

editorOptions: { block: false } + Neos.Fusion:Editable { } | { block: true } -> <div>{isInlineMode: true}</div>
editorOptions: { block: true } + Neos.Fusion:Editable { block: false } -> <span>{isInlineMode: false}</span>

So the the notation of block: false will always overrule the one from the Neos.Fusion:Editable. But the rendering will still be affected via Neos.Fusion:Editable
(i hope i have no logic error in my equations ^^)

@mhsdesign mhsdesign marked this pull request as draft August 7, 2023 13:04
The outer tag is not removed anymore - instead youll get a span. You should switch to the new isInline mode
@mhsdesign mhsdesign changed the base branch from 8.3 to 8.4 January 14, 2024 18:25
@github-actions github-actions bot added 8.4 and removed 8.3 labels Jan 14, 2024
@grebaldi grebaldi linked an issue Jan 15, 2024 that may be closed by this pull request
Copy link
Contributor

@grebaldi grebaldi left a comment

Choose a reason for hiding this comment

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

Hi @mhsdesign,

thank you for tackling this problem :)

I tested your changes locally and I noticed a problem with the placeholders:

before after
Peek 2024-01-15 16-39 Peek 2024-01-15 16-41

Editor initialization and editing itself are working fine. Only the placeholder is missing - which makes the editor virtually unusable.

I used this setup for reproduction:

NodeType
'Vendor.Site:Content.Headline':
  superTypes:
    'Neos.Neos:Content': true

  ui:
    icon: header
    label: 'Headline'

  properties:
    type:
      type: string
      ui:
        label: 'Headline Type'
        reloadIfChanged: true
        inspector:
          editor: 'Neos.Neos/Inspector/Editors/SelectBoxEditor'
          editorOptions:
            values:
              h1:
                label: 'H1'
              h2:
                label: 'H2'
              h3:
                label: 'H3'
              h4:
                label: 'H4'
              h5:
                label: 'H5'
              h6:
                label: 'H6'
    text:
      type: string
      ui:
        inlineEditable: true
        inline:
          editorOptions:
            placeholder: '- Insert Text here -'
            isInline: true
            linking:
              anchor: true
              title: true
              relNofollow: true
              targetBlank: true
            formatting:
              strong: true
              em: true
              u: true
              sub: true
              sup: true
              del: true
              p: false
              h1: false
              h2: false
              h3: false
              h4: false
              h5: false
              h6: false
              pre: false
              underline: true
              strikethrough: true
              removeFormat: true
              left: true
              right: true
              center: true
              justify: true
              table: false
              ol: false
              ul: false
              a: true
Fusion
prototype(Vendor.Site:Content.Headline) < prototype(Neos.Neos:ContentComponent) {
  type = ${q(node).property('type') || 'div'}

  text = Neos.Neos:Editable {
    property = 'text'
    block = false
  }

  renderer = Neos.Fusion:Tag {
      tagName = ${props.type}
      content = ${props.text}
    }
}

Another issue:

We decided to use the the same naming as in fusion: block false.

☝️ this is not yet reflected in code afaict. The editorOptions configuration still says isInline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.4 Feature Label to mark the change as feature Wrong Branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FEATURE: Proper inline mode (autoparagraph "false")
2 participants