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

Adding styling to a mention built with MentionCustomization adds the style tags in the wrong place #6587

Closed
adampal opened this issue Apr 9, 2020 · 6 comments · Fixed by #6739
Assignees
Labels
package:mention type:docs This issue reports a task related to documentation (e.g. an idea for a guide).

Comments

@adampal
Copy link

adampal commented Apr 9, 2020

📝 Provide detailed reproduction steps (if any)

The bug occurs any time when you use the mention feature with the MentionCustomization function.
You can see the bug in action in the ckeditor docs page on the example here:

In that demo,

  1. Highlight the @tdog text .
  2. Click the Bold button to add the bold style to the text
  3. Inspect the element

✔️ Expected result

What is the expected result of the above steps?

The expected result is that the <strong> tags should be added outside of the <span> that creates the mention.
Eg <strong><span>my mention</span></strong>
This works correctly when you do the same actions with the demo at the top of the page here

❌ Actual result

What is the actual result of the above steps?

What actually happens is that the <strong> tag is inside the <span> tag
Eg <span><strong>my mention</strong></span>

This creates an issue when pulling data back from a database to re-render the mention. It seems like because the contents of the <span> tag don't match up with what is expected, it gets flagged as a partial mention and the mention span is not rendered.

I'm thinking maybe here in the source code is where the issue happens.

📃 Other details

  • Browser: Safari 13.0.5
  • OS: MacOS 10.15.3
  • CKEditor version: 18.0.0
  • Installed CKEditor plugins:
    EssentialsPlugin,
    UploadAdapterPlugin,
    AutoformatPlugin,
    BoldPlugin,
    ItalicPlugin,
    Underline,
    BlockQuotePlugin,
    EasyImagePlugin,
    HeadingPlugin,
    ImagePlugin,
    ImageCaptionPlugin,
    ImageStylePlugin,
    ImageToolbarPlugin,
    ImageUploadPlugin,
    LinkPlugin,
    ListPlugin,
    ParagraphPlugin,
    Mention,
    MentionCustomization,
    Table,
    TableToolbar,
    TableProperties,
    TableCellProperties,
    Indent,
    IndentBlock,
    RemoveFormat,
    Font,
    Highlight,
    HorizontalLine,
    PasteFromOffice,
    Alignment,
    PageBreak,
    Subscript,
    Superscript,

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@adampal adampal added the type:bug This issue reports a buggy (incorrect) behavior. label Apr 9, 2020
@Mgsy
Copy link
Member

Mgsy commented Apr 14, 2020

Hi! Thanks for the report, I can reproduce the issue. After loading the content with a styled customized mention, the marker is lost.

Steps to reproduce:

  1. Go to http://localhost:8125/ckeditor5-mention/tests/manual/mention-custom-view.html
  2. Add bold to one of the markers
  3. editor.setData( editor.getData() )

I've noticed also another behaviour - styles applies to the whole marker regardless of the selection. If you'll style only one character, the whole marker becomes styled.

@Reinmar
Copy link
Member

Reinmar commented Apr 14, 2020

@jodator, AFAICS MentionEditing uses priority:20 for the view attribute element generated for the mention. Shouldn't we use the same priority in the other guides and manual tests?

@jodator
Copy link
Contributor

jodator commented Apr 15, 2020

Shouldn't we use the same priority in the other guides and manual tests?

Yep. Confirmed - we should use it also in the docs and manual tests.

@jodator jodator added package:mention type:docs This issue reports a task related to documentation (e.g. an idea for a guide). and removed type:bug This issue reports a buggy (incorrect) behavior. labels Apr 15, 2020
@jodator jodator self-assigned this Apr 15, 2020
@adampal
Copy link
Author

adampal commented Apr 23, 2020

I've noticed also another behaviour - styles applies to the whole marker regardless of the selection. If you'll style only one character, the whole marker becomes styled.

@Mgsy I like this particular behaviour. It's what I was expecting to happen as a user.

@adampal
Copy link
Author

adampal commented Apr 23, 2020

Thanks for looking in to the issue everyone.
To confirm, would changing the priority change the order of the html tags?

@jodator
Copy link
Contributor

jodator commented Apr 24, 2020

@adampal yes you need to change attribute element priority. This is a default mention downcast:

https://github.com/ckeditor/ckeditor5-mention/blob/572d198f8beef66e10d8ed7f14420a19f13d34cc/src/mentionediting.js#L134-L150:

viewWriter.createAttributeElement( 
    'span',
    {
        class: 'mention',
        'data-mention': mention.id
    }, 
    {
        id: mention._uid,
        priority: 20 // <-- attribute element priority.
    } 
);

@jodator jodator added this to the iteration 31 milestone Apr 27, 2020
@mlewand mlewand modified the milestones: iteration 31, iteration 32 May 4, 2020
Reinmar added a commit that referenced this issue May 22, 2020
Docs (mention): Add notes about using `AttributeElement`'s priority and id attribute in mentions. Closes #6587.

Other (mention): Renamed `MentionAttribute._uid` to `MentionAttribute.uid` as it needs to be used by integrators when implementing custom converters. Closes #6587.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:mention type:docs This issue reports a task related to documentation (e.g. an idea for a guide).
Projects
None yet
5 participants