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

fix info-page issues #6100

Merged
merged 1 commit into from
Feb 18, 2025
Merged

fix info-page issues #6100

merged 1 commit into from
Feb 18, 2025

Conversation

hom3mad3
Copy link
Contributor

@hom3mad3 hom3mad3 commented Feb 18, 2025

fixes #6071, #6082 and #6072

Copy link
Collaborator

@vellip vellip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM just one question

@@ -1,5 +1,5 @@
{% extends 'base.html' %}
{% load i18n %}
{% load i18n ckeditor_tags %}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? As far as I know, safe is built into django?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's needed so the collapsible element (accordion) renders correctly.
as a matter of fact i forgot one extra tag (disable_iframes)

@goapunk's explanation:

yes it makes the accordions (called collapsibles in our code for some reason) render correctly

so it's only needed in places the ckeditor allows adding accordions

and there's a tag for places where the ckeditor allows embeding videos called disable_iframes which prevents the videos from auto-loading and leaking data to youtube before agreeing to the video opt in

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i forgot to explicitly call the collapsible on the template too 🙈

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that explains my confusion :D

@hom3mad3 hom3mad3 force-pushed the 2025-02-fix-info-page branch from 83de54a to b8a333b Compare February 18, 2025 12:51
@hom3mad3 hom3mad3 removed the request for review from sevfurneaux February 18, 2025 12:52
@hom3mad3 hom3mad3 merged commit 25ebce0 into dev Feb 18, 2025
2 checks passed
@hom3mad3 hom3mad3 deleted the 2025-02-fix-info-page branch February 18, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants