umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.47k stars 2.69k forks source link

RTE Style format change prevents using both appsettings and stylesheet in a single RTE #14819

Open matthewcare opened 1 year ago

matthewcare commented 1 year ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

12.1

Bug summary

I've just upgraded a site to v12, and there are issues with the style selector which appear to have been introduced by this PR https://github.com/umbraco/Umbraco-CMS/pull/13582

It is now not possible to define styles in both a stylesheet and in appsettings for a single RTE.

Specifics

I was using the Umbraco:CMS:RichTextEditor:CustomConfig:style_formats option to add additional styles to my RTE which aren't possible do to inside Umbraco.

Because I use a stylesheet in addition to the custom style formats, the custom style formats no longer show. If I remove the stylesheet from the RTE and rely on the style formats value, then they do show, but then the stylesheet isn't loaded and you don't get a visual for the styles.

The reason for not only using the stylesheet option is because you can't do "complex" things like applying multiple classes to specific elements.

Using the following stylesheet contents, and appsettings value, I was able to have more configuration for my formats, which is now not possible.

.link-heading {
    color: inherit;
    padding-bottom: 4px;
    border-bottom: 1px solid;
    text-decoration: none;
}

.list {
    padding: 0;
    list-style: none;
}

.list-check > * {
    padding-left: 30px;
}

.list-check > :before {
    content: "";
    display: list-item;
    position: relative;
    left: -30px;
    width: 30px;
    height: 1.5em;
    margin-bottom: -1.5em;
    background-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='24' height='24'%3E%3Cpath fill='%233C3C3B' fill-rule='evenodd' d='M11.5 4c2 0 3.8.7 5.3 1.8l-1.5 1.5a6.5 6.5 0 1 0 2.6 6.2l2-2A8.5 8.5 0 1 1 11.5 4zm9.2 2.3a1 1 0 0 1 0 1.4l-8 8a1 1 0 0 1-1.4 0l-4-4a1 1 0 0 1 1.4-1.4l3.3 3.3 7.3-7.3a1 1 0 0 1 1.4 0z'/%3E%3C/svg%3E");
    background-repeat: no-repeat;
    background-position: 50% 50%;
}

.list-decimal {
    counter-reset: decimal;
}

    .list-decimal > * {
        padding-left: 50px;
        counter-increment: decimal;
    }

    .list-decimal > ::before {
        background-color: #f6f4f0;
        border-radius: 50%;
        color: #8a704c;
        content: counter(decimal, decimal);
        font-size: 1.25rem;
        font-weight: 500;
        height: 32px;
        left: -50px;
        line-height: 28px;
        text-align: center;
        width: 32px;
        margin-bottom: -1.5em;
        display: list-item;
        list-style-position: inside;
        position: relative;
    }

/**umb_name:Lead Text*/
.text-lead {
    font-size: 20px;
    line-height: 1.5;
    color: #8a704c;
}

/**umb_name:H2*/
h2 {
    font-size: 24px;
    font-weight: bold;
}

/**umb_name:H3*/
h3 {
    font-size: 19px;
    font-weight: bold;
}

/**umb_name:H4*/
h4 {
    font-size: 16px;
    font-weight: bold;
}
"RichTextEditor": {
  "CustomConfig": {
    "style_formats": "[{\"title\":\"Checked List\", \"selector\": \"ul\", \"classes\": \"list list-check\" },{\"title\":\"Numbered List\", \"selector\": \"ol\", \"classes\": \"list list-decimal\" },{\"title\":\"Heading Link\", \"selector\": \"a\", \"classes\": \"link-heading\" }]"
  }
},

As a workaround, I removed the comments from the stylesheet, and defined them in appsettings, so now technically there are no style formats in the stylesheet, but it still loads and applies the styles to my formats.

While I understand the reasoning behind the PR, and agree that not being able decide which RTEs receive the appsetting formats, this is a breaking change with an inelegant workaround.

Steps to reproduce

Using the example data in the specifics, create a stylesheet, and also an appsettings value which both define style formats. Observe that an RTE which has a stylesheet applied won't be able to have any formats from appsettings.

Expected result / actual result

Expected:

I should be able to have both appsettings and stylesheet formats, or be able to choose one or both.

Actual:

I am only able to have one or the other.

github-actions[bot] commented 1 year ago

Hi there @matthewcare!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

netcamo commented 1 year ago

Hi @matthewcare,

Thank you for reaching out with this issue! 😄

As described in the PR that you have mentioned on the issue, it has been decided that it should not be possible to mix and match any longer.

It is recommended for you to build the styles in the RTE stylesheets. It is possible to include multiple stylesheets in an RTE, meaning you could build your "base" styles in one stylesheet and retain the special (the per-RTE-instance) in your current ones. You'd then have to include the "base" stylesheet in the relevant RTE configs (as well as the special ones).

I will close this issue as it is expected behaviour for now, but if you feel that this need to be changed again in the future please feel free to open a discussion on it to gather feedback from the greater community 😄

Thank you again for reaching out to us! Have a wonderful day! 😄

matthewcare commented 1 year ago

Hi @netcamo, Though I appreciate this is "decided" functionality, it is stil a breaking change with no solution. There are things you can do in style_formats that you cannot do in stylesheets. So although it is recommended to build the styles in RTE stylesheets, is it not possible in all scenarios. There are also things you cannot do in style_formats that you can only do in RTE stylesheets - such as styling the custom style formats.

I'm happy to open a discussion for feedback, but I'm looking for a solution to a problem which now exists without having to rollback to a previous version.

Alternatively I'll add it to my list of future PRs.

Thanks, Matthew

TobyPerplex commented 2 months ago

Hi @netcamo, I work at Perplex and we used to be able to do "style_formats": "[ { \"title\": \"Checklist\", \"selector\": \"ul\", \"classes\": \"checklist\" }]" in the appsettings.json and

ul.checklist {
    list-style: none;
}

ul.checklist li {
    position: relative;
}

ul.checklist li::before {
    content: '✔';
    position: absolute;
    inset-inline-start: -20px;
}

in the stylesheet to have a checklist appear in tinymce's formats and when applied to a bulletlist would have some basic styling, it worked perfectly! And would result in:

Screenshot 2024-08-08 at 17 23 33

or this in the DOM:

<ul class="checklist">
<li>List item 1</li>
<li>List item 2</li>
<li>List item 3</li>
</ul>

Now this is no longer possible because we can't use both appsettings and a stylesheet for a single RTE anymore.

If I now only use appsettings the checklist wouldn't show some basic styling or how @matthewcare worded it There are also things you cannot do in style_formats that you can only do in RTE stylesheets - such as styling the custom style formats..

Alternatively, if I now only use a stylesheet and add /**umb_name:Checklist*/ above ul.checklist { and then apply formatting to a bulletlist I'll get a ul inside a ul (similar to #12036):

<ul class="checklist">
<ul>
<li>List item 1</li>
<li>List item 2</li>
<li>List item 3</li>
</ul>
</ul>

If I remove ul. from the styling I'll get 3 span tags with a checklist class:

<ul>
<li><span class="checklist">List item 1</span></li>
<li><span class="checklist">List item 2</span></li>
<li><span class="checklist">List item 3</span></li>
<ul>

Can you re-open this issue and think of a solution that fixes https://github.com/umbraco/Umbraco-CMS/issues/13577 but still makes it possible to use both appsettings and a stylesheet for a single RTE?

JamieT-Shout commented 1 week ago

Just to pipe in that we require this as well, we are currently upgrading a lot of websites to v13 and functionality the client has already on v8 such as the style options combined from the appsetting and the css file as like others have mentioned, there are features that do not work via the CSS option and require the "style_formats".

Perhaps a setting could be introduced which allows us to combine or not? Best of both worlds.