django-cms / djangocms-frontend

django CMS frontend is a plugin bundle for django CMS providing several components for the frontend, currently implemented with the popular Bootstrap 5 framework.
Other
55 stars 22 forks source link

bug: frontend icons not visible if within text plugin #115

Closed TLuesebrinck closed 1 year ago

TLuesebrinck commented 1 year ago

The settings flag DJANGOCMS_FRONTEND_USE_ICONS do not enable links with icons. I can find any code where this flag is checked.

How do I make icons in general visible in the editor?

fsbraun commented 1 year ago

The DJANGOCMS_FRONTEND_USE_ICONS currently only allows for djanogcms-icon icons. To change this, there's a PR for this: https://github.com/django-cms/djangocms-frontend/pull/113

Feel free to test it: pip install git+https://github.com/fsbraun/djangocms-frontend@fix/link_icons

TLuesebrinck commented 1 year ago

Ok, this works now. The link icon is not visible in the ckeditor before save, but that ok. The other icons from the icon plugin are now visible in the ckeditor, I didn't expect that.

fsbraun commented 1 year ago

True. I renamed the issue. This needs a fix.

TLuesebrinck commented 1 year ago

I have moved my page to a Debian server with python 3.9. Using the current version with the icon fix, crash the server (error 500) when I open a page, where I use a link with an icon. With the release build 1.1.0, no crash.

fsbraun commented 1 year ago

Can you provide a traceback? What else has changed (except moving to Python 3.9)?

TLuesebrinck commented 1 year ago

On my local system, I used Python 3.11, we could not get it working on Debian, so we just use 3.9 from Debian 11. However, there is no debug trace, it just says server error 500 not more. I also checked the logs I am aware of but no one show any problem. There is something suspicious. The links with icons show some data in the class attribute which look not right, especially in the not edit mode.

<a href="https://" class="btn-success d-block btn mb-3"><i class="{'libraryId': 'bi', 'libraryName': 'bootstrapIcons', 'iconHtml': '<i class=&quot;bi bi-download&quot;></i>', 'iconMarkup': '&amp;lt;i class=&amp;quot;bi bi-download&amp;quot;&amp;gt;&amp;lt;/i&amp;gt;', 'iconClass': 'bi bi-download', 'iconText': '', 'library': 'bootstrap-icons'} " aria-hidden="true"></i>
Download (includes free trial)</a> 
fsbraun commented 1 year ago

@TLuesebrinck Can you try adding a config in your setup that will send you the traceback by email? Something like:

LOGGING = {
    'version': 1,
    'disable_existing_loggers': False,
    'filters': {
        'require_debug_false': {
            '()': 'django.utils.log.RequireDebugFalse',
        },
        'require_debug_true': {
            '()': 'django.utils.log.RequireDebugTrue',
        },
    },
    'formatters': {
        'django.server': {
            '()': 'django.utils.log.ServerFormatter',
            'format': '[{server_time}] {message}',
            'style': '{',
        }
    },
    'handlers': {
        'console': {
            'level': 'INFO',
            'filters': ['require_debug_true'],
            'class': 'logging.StreamHandler',
        },
        'django.server': {
            'level': 'INFO',
            'class': 'logging.StreamHandler',
            'formatter': 'django.server',
        },
        'mail_admins': {
            'level': 'ERROR',
            'filters': ['require_debug_false'],
            'class': 'django.utils.log.AdminEmailHandler',
            'include_html': True
        }
    },
    'loggers': {
        'django': {
            'handlers': ['console', 'mail_admins'],
            'level': 'INFO',
        },
        'django.server': {
            'handlers': ['django.server'],
            'level': 'INFO',
            'propagate': False,
        },
    }
}
TLuesebrinck commented 1 year ago

I think my Debian is not setup to send mails. However, I add a file writing handler, but the file it creates is empty.

fsbraun commented 1 year ago

Sorry, I cannot reproduce the issue. Which version of djangocms-frontend are you using exactly?

TLuesebrinck commented 1 year ago

In general, version 1.1.0 but because of the missing icons, I tried the version with the latest fixes. Maybe I install it in the wrong way, the icon option in the link/button plugin is not there.

fsbraun commented 1 year ago

How did you install? I'd assume pip install git+https://github.com/fsbraun/djangocms-frontend@fix/link_icons.

TLuesebrinck commented 1 year ago

yes, I tried also this.

TLuesebrinck commented 1 year ago

I got it working. But I cannot say what caused it, or what have solved the problem.

TLuesebrinck commented 1 year ago

There is still something wrong. I cannot save a link plugin it always output "Please correct the error below.", but there is nothing marked below.

In the console log I found this: It tries to load a css file which not exists: http://127.0.0.1:8000/static/cms/css/3.11.1/cms.icons.css Also in the console log : cannot read properties of undefined (reading 'define') if (jQuery && jQuery.fn && jQuery.fn.select2 && jQuery.fn.select2.amd) var e = jQuery.fn.select2.amd; e.define("select2/i18n/en", [], function() {

TLuesebrinck commented 1 year ago

The main issue (can't save link plugins) above is caused by Django 4.2 version 4.1.7 is fine, but the latest release break something.

fsbraun commented 1 year ago

Django 4.2? Not compatible yet.

fsbraun commented 1 year ago

@TLuesebrinck If you update djangocms-frontend's dependency django-entangled to at least versione 0.5.4 djangocms-frontend will now run under Django 4.2. 😎

corentinbettiol commented 1 year ago

We're having the same problem (happened on prod when we tried to create a new Link plugin):

Missing staticfiles manifest entry for 'cms/css/3.9.0/cms.icons.css'

Using django 3.2, django-cms 3.9.X, djangocms-icon 2.0.0, and c4d6dac3657f3f8d4867ed8c71da75825d2116fc (latest commit on branch master yet).

There's no cms.icons.css file in our static folder. The problem seems to be this call?

fsbraun commented 1 year ago

@corentinbettiol I will look into this. Indeed, it seems that the call is a copy-paste error. This is a system resource not available in any official django cms release. The line probably needs to go away.

But then again, I do not assume it leads to any complication besides the console error.

If you want to use djangocms-icon please remove djangocms-frontend.contrib.icon from your installed apps. This will load djangocms-icon's icon picker into the link plugin.

With djangocms-frontend.contrib.icon in the installed apps, link icons should work fine. Also, icons within text plugins work fine - except when editing. This needs to be solved and is an open issue right now. Hope to find a solution soon.

fsbraun commented 1 year ago

@corentinbettiol @TLuesebrinck I've tested with both djangocms-icon and djangocms-frontend.contrib.icon: In both cases the icon is not visible in the editor. Instead, ckeditor removes the empty span of the icons and treats the text plugin as a block type plugin leading to a line break. Can you confirm?

I am tempted to move that issue to djangocms-text-ckeditor since it happens for both djangocms-icon and djangocms-frontend.

fsbraun commented 1 year ago

Turns out that this has been discussed a few times for djangocms-text-ckeditor (see, e.g., here: https://github.com/django-cms/djangocms-text-ckeditor/issues/408). To this end, I added a description on how to configure ckeditor correctly to display icons in #122

corentinbettiol commented 1 year ago

The fix described in #122 worked, the icon picker is working in djangocms-frontend 1.1.1 (and is very nice), thanks!

corentinbettiol commented 1 year ago

Just adding an answer to this (in one of your previous comment):

But then again, I do not assume it leads to any complication besides the console error.

That was a server error actually. The staticfiles app was really not happy with a missing file.

We had to deactivate the feature (comment djangocms_frontend.contrib.icon in INSTALLED_APPS) to make the link plugin work again (before updating djangocms-frontend).

fsbraun commented 1 year ago

Thank you. Some static file configurations throw an error on missing files, that's correct. (But then you do not get an error in the browser console.) djangocms-frontend works with both djangocms-icons and djangocms-frontend.contrib.icon. But only one icon plugin can be installed due to naming conflicts.

corentinbettiol commented 1 year ago

I updated our staging and... it's still not working. At least the server does not return a 500 anymore.

The url that the icon app tries to fetch to get the icons is https://static.our-domain.ext/djangocms_frontend/icon/vendor/assets/js/universal-icon-picker.min.ac1a8d2c2564.jsicons-libraries/font-awesome.min.json.

This url is indeed a 404, the real url is https://static.our-domain.ext/djangocms_frontend/icon/vendor/assets/js/universal-icon-picker.min.ac1a8d2c2564.js (and some js code is found, as intended), and then icons-libraries/font-awesome.min.json seems to be pasted right after the url.

Here's our ckeditor settings for this project:

CKEDITOR_SETTINGS = {
    "language": "",
    "colorButton_colors": "FFF,000,00A5DF,1A1A1A,FAC800,F39200,00652C,B94420,878C91,6EB5BD",
    "stylesSet": [
        {
            "name": "First paragraph",
            "element": "p",
            "attributes": {"class": "intro"},
        },
        f"default:{STATIC_URL}djangocms_frontend/icon/ckeditor/ckeditor.icons.js",  # this is what I added this morning (it's working in our dev env)
    ],
    "contentsCss": f"{STATIC_URL}css/ckeditor_theme.min.css",
    "format_tags": "p;h1;h2;h3;h4;h5;h6",
    "pasteFilter": "plain-text",
    "removeButtons": "PasteText,PasteFromWord,Maximize,Link,Unlink",
}

I even added an example config to load only certain icon picker:

DJANGOCMS_FRONTEND_ICON_LIBRARIES = {
    'font-awesome': (
        'font-awesome.min.json',
        'https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.0.0/css/all.min.css'
    ),
    'bootstrap-icons': (
        'bootstrap-icons.min.json',
        'https://cdn.jsdelivr.net/npm/bootstrap-icons@1.10.3/font/bootstrap-icons.css'
    ),
    'material-icons-filled': (
        'material-icons-filled.min.json',
        'https://fonts.googleapis.com/css2?family=Material+Icons'
    ),
}

If I switch to another icon set and click the "click to add icon" button, the dev tools shows that another url was not found (replacing icons-libraries/font-awesome.min.json by icons-libraries/bootstrap-icons.min.json or anything else).

Did I miss something?

fsbraun commented 1 year ago

This looks like an invalid assumption on the path of the icon library files somewhere. I will need to look.

fsbraun commented 1 year ago

OK, first research makes it obvious that the corresponding icon picker library assumes about how files need to be loaded. The good news is: This is not necessary. I'd suggest that I create a PR which will patch the icon library and will provide Django's full static paths to the icon library. If you can test this and it works, I'll also create a PR for the icon picker to get this implemented permanently.

corentinbettiol commented 1 year ago

Yeah I can test this on a staging environment! Thanks!

fsbraun commented 1 year ago

@corentinbettiol Can you test with this update: https://github.com/fsbraun/djangocms-frontend/tree/fix/respect_django_static_path (also in this PR #123 )

corentinbettiol commented 1 year ago

@fsbraun Awesome, it works!

image

fsbraun commented 1 year ago

OK, but the labels on the left seem strange... 🤪. Also the "search" word seems strange. This is probably another assumption.

corentinbettiol commented 1 year ago

Yeah it seems like it picks the file name (and the staticfiles app adds a md5 hash to the file name).

corentinbettiol commented 1 year ago

The "search" word is the alt for this img: https://static.our-domain.ext/djangocms_frontend/icon/vendor/assets/js/universal-icon-picker.min.e01bd5d5b1d1.js/images/magnifying-glass-solid.svg

I don't think /images/magnifying-glass-solid.svg should be added to this url :P


edit: (and the "close" word is the alt text for https://static.our-domain.ext/djangocms_frontend/icon/vendor/assets/js/universal-icon-picker.min.e01bd5d5b1d1.js/images/xmark-solid.svg)

fsbraun commented 1 year ago

Another update ... The names are not fixed yet. The clean solution would be to include them in the json files.

corentinbettiol commented 1 year ago

image

The icons are displaying fine now :)

fsbraun commented 1 year ago

@corentinbettiol Can you do a final check - now with the library names? Thanks so much!

corentinbettiol commented 1 year ago

@fsbraun Sorry I finished work minutes before your comment :/

The problem with the library names is fixed! The icon picker problems are all solved (for our usecases)! Big thanks for your responsiveness!

image

Have you planned a date to release an update of the package on pypi ? (we prefer to use the classic versions of the packages on our productions rather than using commits)

fsbraun commented 1 year ago

I'm glad you discovered this! Thanks so much for your support! I will wait until I release the next update to catch potential other issues, if that's ok with you.

corentinbettiol commented 1 year ago

Yeah no problem, we're going to use your branch until the new version is published :)

fsbraun commented 1 year ago

@corentinbettiol 1.1.2 out now. (or better as soon as pypi maintenance is over)

corentinbettiol commented 1 year ago

Awesome! Thanks a lot!