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: restore auto-paragraph #412

Merged
merged 4 commits into from
Jan 30, 2024
Merged

fix: restore auto-paragraph #412

merged 4 commits into from
Jan 30, 2024

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Jan 18, 2024

Overview

Allow WYSIWYG to force <p> tags.

Related

Changes

  • added CKEDITOR_SETTINGS from CMS
  • changed one CKEDITOR_SETTINGS value from CMS

Testing

Auto-Paragraph Feature

  1. Create a Text plugin instance.
  2. Be on a line that does not have a block level element yet (no <p>, no <h1>):
    1. Delete any existing content.
    2. Press backspace again to remove empty block-level element (e.g. <h1>, <p>).
  3. Choose an inline element from the Styles dropdown e.g. "Big".
  4. Save Text content.
  5. Confirm rendered markup does not wrap the inline element with a <p> tag.

Auto-Paragraph Side-Effects

  1. Find a card pattern that has inline elements on their own line, e.g.
  2. Edit that Text plugin instance.
  3. View WYSIWYG source editor.
  4. Verify inline element was auto-wrapped in a <p> tag.
  5. Save.
  6. Compare style of card to screenshot.
  7. Verify content of the Text has:
    • either not changed in style
    • or not changed enough to be a problem
  8. Repeat for other types of unwrapped inline elements.

UI

Auto-Paragraph Feature

Before.mov

Auto-Paragraph Side-Effects

Before (sans <p>) After (with <p>)
a.c-button button BEFORE button AFTER
a > i link wth icon BEFORE link wth icon AFTER
I asked designers…
ask designers
M.S. just worries about orphans (text that wraps a single word). No one card style can prevent that. That's up to content editors.
2 link buttons in 1 p tag butt against each other when they wrap
2 links in 1 p - code
when link buttons wrap and when they do not
2 links in 1 p do wrap 2 links in 1 p no wrap

Copy link
Member Author

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

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

Notes for reviewers.

Comment on lines +211 to +213
'stylesSet': 'default:/static/js/addons/ckeditor.wysiwyg.js',
'contentsCss': ['/static/djangocms_text_ckeditor/ckeditor/contents.css'],
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Explanation. I must duplicate these two settings from Core-CMS, because settings are not smart enough to merge lists.

@@ -205,6 +205,12 @@
# https://github.com/django-cms/django-filer/blob/2.0.2/docs/permissions.rst
FILER_ENABLE_PERMISSIONS = True

# https://github.com/django-cms/djangocms-text-ckeditor
CKEDITOR_SETTINGS = {
'autoParagraph': True, # Core-CMS had set this to False
Copy link
Member Author

Choose a reason for hiding this comment

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

Important. This is what is changing. Paragraphs will automatically wrap inline elements upon editing a Text plugin instance.

@wesleyboar
Copy link
Member Author

wesleyboar commented Jan 30, 2024

@R-Tomas-Gonzalez and I double-tested on local and dev.

A good page to test was https://dev.tup.tacc.utexas.edu/use-tacc/getting-started/.

@wesleyboar wesleyboar merged commit a378753 into main Jan 30, 2024
1 check passed
@wesleyboar wesleyboar deleted the fix/restore-auto-paragraph branch January 30, 2024 22:17
wesleyboar added a commit that referenced this pull request Feb 16, 2024
#412 Side Effect:
- A `<p>` is added around any Text within a `c-feed-list`.

This commit:
- Add support for time wrapped in `<p>`.
- Retain support for time NOT wrapped in `<p>`.
wesleyboar added a commit that referenced this pull request Feb 19, 2024
* hotfix: c-feed-list must support `<p><time/></p>`

#412 Side Effect:
- A `<p>` is added around any Text within a `c-feed-list`.

This commit:
- Add support for time wrapped in `<p>`.
- Retain support for time NOT wrapped in `<p>`.

* hotfix: cleanup and fix c-feed-list further

* hotfix: no margin bottom on c-feed-list time & a

* hotfix: no margin bottom on c-feed-list time & a … for real

* hotfix: remove unnecessary CSS selector
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