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

[Data Objects] WYSIWYG data-type: use <br> instead of <p> #4411

Closed
BlackbitDevs opened this issue May 21, 2019 · 20 comments
Closed

[Data Objects] WYSIWYG data-type: use <br> instead of <p> #4411

BlackbitDevs opened this issue May 21, 2019 · 20 comments

Comments

@BlackbitDevs
Copy link
Contributor

Currently the inserted text of an wysiwyg field of a data object gets wrapped in a p-tag by default. We can disable this by setting editor configuration of the field definition to

{
  enterMode:2
}

In our experience only few customers are aware of this "feature", most just recognize it when output layout gets scrambled.

Imho it would be better to set enterMode: 2 by default as it is much easier to wrap the output in a p-tag whereever needed than to remove the p-tag afterwards.

What do you think?

@brusch brusch self-assigned this Jun 4, 2019
@brusch brusch added this to the 6.0.0 ("Ale") milestone Jun 4, 2019
@brusch brusch changed the title Default enterMode: 2 for wysiwyg fields of data objects [Data Objects] WYSIWYG data-type: use <br> instead of <p> Jun 5, 2019
@brusch brusch closed this as completed in 32adcab Jun 5, 2019
@ckemptner
Copy link
Contributor

ckemptner commented Jul 9, 2019

Please undo this change asap. It is now not possible anymore to add a new paragraph by hitting the "Enter" key. (Which is the default text editing behavior in all major text editing software such as Word or Wordpress - Why make it different in Pimcore than how most users know it?)
I would expect that such a change gets voted on by asking at least some community content editors -before implementing it - as this is a major change in the of content editor experience.

See also here
https://ckeditor.com/docs/ckeditor4/latest/features/enterkey.html

"Changing the Enter Mode setting to BR or DIV is not recommended. The default CKEDITOR.ENTER_P mode is fully supported by all editor features and plugins and is also the most correct one in terms of best practices for creating web content."

Or here
ckeditor/editor-recommendations#36

@NiklasBr
Copy link
Contributor

NiklasBr commented Jul 9, 2019

I'm in agreement with @ckemptner

@brusch
Copy link
Member

brusch commented Jul 9, 2019

Feel free to continue the discussion, until v7 we have some time left.

@BlackbitDevs
Copy link
Contributor Author

BlackbitDevs commented Jul 9, 2019

The question is what should be default behaviour. Certainly there are cases where p tags make sense.

If you want the old behaviour with p-tags back you can enter
{ enterMode:1 }
in the wysiwyg field's editor configuration.

But for all cases where users just want to enter some text and format it, p-tags can be counter intuitive in my (and a lot of our customers') experience.

@katharina-kopf
Copy link

Please undo this change asap! I work a lot in pimcore and I prefer adding a paragraph by hitting the "Enter" key - the same way as it works in word!

@NiklasBr
Copy link
Contributor

NiklasBr commented Jul 9, 2019

p-tags can be counter intuitive in my (and a lot of our customers') experience.

This is where my experience is the exact inverse of yours, customers expect consistency and paragraphs. And the default behaviour of other systems is now different from Pimcore.

@BlackbitDevs
Copy link
Contributor Author

BlackbitDevs commented Jul 9, 2019

There are 2 problems which initially got me to issue this:

  1. When you only have one line in a wysiwyg field, e.g. This is <b>bold</b> and press save it gets converted to <p>This is <b>bold</b></p>. It is not possible to enter something without the p-tags - they always got wrapped around the content.
  2. When you set a wysiwyg field's content by code, e.g. $object->setXyz('This is <b>bold</b>'); the content in the database really is This is <b>bold</b> but when you later open the object in the Pimcore admin GUI and save the object, it gets converted as in 1. Especially this behaviour is problematic because nobody actually changed the content of the field via GUI and nevertheless its content in the database changed.

It would be fine if p-tags where only added if really "enter" got pressed in the GUI - but if you only enter one line, p-tags should not be added automatically.
And contents must not be altered on saving via GUI if the field has not been touched.

If above problems do not apply for your case and you do want to add a paragraph by pressing "enter", then simply add { enterMode:1 } in the wysiwyg field's editor configuration.

Perhaps we could add a little more documentation about this or even add a dropdown in the wysiwyg field definition to set the enter mode.

@jansarmir
Copy link
Contributor

I think, everyone will later customize editor for what he is expecting
making frontend design consistent, you want to remove h1 tag mainly and styles too
make one stable decision, and add better example in doc how customize it on you own
like mine example, its easy

"use strict";
// src/AppBundle/Resources/public/js/pimcore/ckeditor.config.js
pimcore.object.tags.wysiwyg.defaultEditorConfig = {
    enterMode: CKEDITOR.ENTER_P,

    format_tags: 'p;h3;h4;h5;h6;pre;div',

    removePlugins: 'stylescombo',
    removeButtons: 'Font'
};
// src/AppBundle/AppBundle.php
class AppBundle extends AbstractPimcoreBundle implements DependentBundleInterface
{
    public function getJsPaths()
    {
        return [
            '/bundles/app/js/pimcore/ckeditor.config.js'
        ];
    }
}

@BlackbitDevs
Copy link
Contributor Author

BlackbitDevs commented Jul 10, 2019

@jansarmir, you are right but this isn't even necessary as you could just write

{
    enterMode: CKEDITOR.ENTER_P,
    format_tags: 'p;h3;h4;h5;h6;pre;div',
    removePlugins: 'stylescombo',
    removeButtons: 'Font'
}

to the wysiwyg field's editor configuration (this is a field in the field definition). Then you would not need any additional files.

I will add a paragraph to the docs...

@brusch
Copy link
Member

brusch commented Jul 11, 2019

If the majority votes for reverting the change (in the next bug-fix release), I'd be happy with that as well, but please, do not complain about SemVer afterwards ...

@NiklasBr
Copy link
Contributor

🙇 thanks

I think starting with semver at v7.0 would be most true to the reality of the situation and easiest to do. Even though I hope it can be introduced earlier (and semver includes provisions for BC breaks and restoring them if done in a minor/patch release).

@BlackbitDevs
Copy link
Contributor Author

To all who want enterMode: CKEDITOR.ENTER_P back as default: Beside that you cannot add a paragraph by pressing the "enter" key, don't you think the points mentioned in #4411 (comment) are problematic for a default setting?

@ckemptner
Copy link
Contributor

Hello Blackbit - Imho for the WYSIWYG editor, which is one of the most-used features of Pimcore users, we have to think "user-first" and not "developer-first". The problems you have mentioned might be solved in some other way - The experience for the users in Pimcore should be consistent with what they know and expect.

@NiklasBr
Copy link
Contributor

NiklasBr commented Jul 11, 2019

don't you think the points mentioned in #4411 (comment) are problematic for a default setting?

Point 1, I think it is an analysis from an incorrect assumption that short paragraphs are not paragraphs. Any and all paragraphs of texts should be wrapped in <p> tags, even if the paragraph is just a short one-line paragraph. It is still a paragraph.

Point 2, yes, this can be problematic, but entirely possible to solve, for example both Drupal and WordPress has had the _filter_autop and wpautop() respectively since basically forever and it works beautifully.

@BlackbitDevs
Copy link
Contributor Author

BlackbitDevs commented Jul 11, 2019

@NiklasBr Of course short paragraphs are paragraphs but what if you want to enter a product name which later on gets output as <h1> or whatever - which actually does not allow a <p>-tag within? Of course you can remove it but which frontend developer will think about this?

And no doubts that the mentioned problems are solvable - and if there is a solution I am the last to stem against CKEDITOR.ENTER_P.
But if you open an object, do nothing and press save there MUST not be a change to the data. Of course this is a developer perspective but when you use enterMode: CKEDITOR.ENTER_P you better be prepared when you imported your data from an external source without <p>-tags and later (after the customer saved some Pimcore objects) get calls why the layout is broken.

@NiklasBr
Copy link
Contributor

NiklasBr commented Jul 11, 2019

but what if you want to enter a product name which later on gets output as <h1> or whatever - which actually does not allow a <p>-tag within

I don't understand why you are using a What You See Is What You Get input field for output where what you get is not what you want to see.

@BlackbitDevs
Copy link
Contributor Author

BlackbitDevs commented Jul 11, 2019

I don't understand why you are using a What You See Is What You Get input field for output where what you get is not what you want to see.

Because the customer wants e.g. to put somethin in italic or bold or change font color of a part of the product name / page title.

@NiklasBr
Copy link
Contributor

For such highly specialised input fields I would configure a very small subset of editor buttons, removing things like images, styles, links, tables and so on to prevent breaking layouts. I can understand that as a valid requirement, but I do not think it is a good reason to set the default behaviour to something non-standard and something which deviates from best practices.

@BlackbitDevs
Copy link
Contributor Author

BlackbitDevs commented Jul 11, 2019

For such highly specialised input fields I would configure a very small subset of editor buttons, removing things like images, styles, links, tables and so on to prevent breaking layouts.

This does not help against <p> tags getting magically added.

Whatever, I have written everything I have to say about this topic. When the majority or the Pimcore team thinks enterMode: CKEDITOR.ENTER_P is a better default, I am fine with this. As described in #4411 (comment) everybody can configure the default behaviour of wysiwyg fields. And if enterMode: CKEDITOR.ENTER_P gets reinroduced as default, I will add this little JS file to future projects and everybody is fine.

@jansarmir
Copy link
Contributor

i think most cases are using CKEDITOR.ENTER_P
if you need other behavior then bunch of html text (mostly content does not care), you can handle it per situation, rather then making global settings

my opinion about wysiwyg field is

  • contains just some html text, always rendered even if its empty, empty

    doesnt effect design

  • but i have option to do some control a settings, if its needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants