search

az-digital / az_quickstart

UArizona's web content management system built with Drupal 10.
https://quickstart.arizona.edu
GNU General Public License v2.0
30 stars 20 forks source link

Arizona Bootstrap loads twice with additional ckeditor 5 config for adding libraries. #2598

Open trackleft opened 1 year ago

trackleft commented 1 year ago

@joshuasosa started review of this PR by successfully testing on the Probo site: enabled CKEditor 5 module, changed Standard text format to use Ckeditor 5, and added styles on the homepage using the new editor.

Interestingly, due to the way CKE5 puts this editor directly into the page compared to CKE4's iframe method, the theme's style.css gets loaded in and applied.

This difference means there's some issues like with bullet points:

CKEditor 4

image

CKEditor 5

image

This natural addition of style.css with the CKE5 editor may be good in a follow-up issue.

Going further though, this means that some styles are already there. It looks like arizona-bootstrap.min.css is already included on the page.

This PR actually includes arizona-bootstrap.min.css a second time: image

It looks like Claro is using the local source while Arizona Barrio allows selection of source and uses CDN by default. I suggest reconciling Claro's Arizona Bootstrap inclusion with this PR's inclusion so the stylesheet isn't loading twice from two different sources unnecessarily.

The az-icons-styles.css does not appear to be included in Claro though, making this PR still relevant for including that stylesheet. But consideration could also be done as to whether the icons stylesheet should be included in Claro if the project ever wants to use those icons elsewhere (like Arizona Quickstart Settings pages) or if it should be left in this PR.

I tested on a new install without this PR's changes and can see arizona-bootstrap.min.css styles being applied successfully with no CKEditor 5-specific changes. Icons though don't show in the editor without this PR.

Vanilla install

image

_Originally posted by @joshuasosa in https://github.com/az-digital/az_quickstart/issues/2447#issuecomment-1565167036_

trackleft commented 1 year ago

Related #807 Related #2596

joeparsons commented 1 year ago

I think the az_paragraphs module's libraries that declare dependencies on various az_barrio libraries (including az_barrio/arizona-bootstrap might be where the existing copy of arizona-bootstrap is coming from.

trackleft commented 1 year ago

I definitely prefer the way that paragraphs adds the library over CKEditor, because it allows the site owner to choose where they get arizona bootstrap from, be it a local copy (preferred these days), or the CDN.

joeparsons commented 1 year ago

The latest CKEditor5 PRs (#2596 and #2769) partially address this but this will likely still be an issue to solve after one of those is merged.