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

Wrap 'card' description with appropriate HTML tags #4360

Merged
merged 3 commits into from
Mar 19, 2020
Merged

Conversation

mmmavis
Copy link
Collaborator

@mmmavis mmmavis commented Mar 17, 2020

Closes #4287

Notes: some vertical spacing around the descrition also got fixed in this PR as before we had redundant empty <p> nodes hanging around. e.g.,
image

Go to the following pages and look for section that looks like
image

Hompage

https://foundation-s-issue-4287-fjnyxw.herokuapp.com/en/

Initiatives

https://foundation-s-issue-4287-fjnyxw.herokuapp.com/en/initiatives/

Participate

https://foundation-s-issue-4287-fjnyxw.herokuapp.com/en/participate/

@patjouk patjouk temporarily deployed to foundation-s-issue-4287-fjnyxw March 17, 2020 19:25 Inactive
@mmmavis mmmavis requested a review from Pomax March 17, 2020 19:44
@@ -9,6 +10,7 @@ def card(image, title, description, link_url, link_label, commitment=None):
'image': image,
'title': title,
'description': description,
'description_is_rich_text': BeautifulSoup(description, 'html.parser').p is not None,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this is a safe way to check this: where is description coming from? Because rich text could also be <ul>...</ul> etc. so we don't want to hardcode that only things with <p> are rich text.

Copy link
Collaborator Author

@mmmavis mmmavis Mar 18, 2020

Choose a reason for hiding this comment

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

Ah agreed that this is not very future proof. I only took CTA's subhead into consideration (passed to template tag as description).

subhead = RichTextField(
        features=[
            'bold', 'italic', 'link',
        ],
        blank=True,
    )

Will update PR.

@mmmavis mmmavis temporarily deployed to foundation-s-issue-4287-fjnyxw March 18, 2020 22:42 Inactive
@mmmavis mmmavis requested a review from Pomax March 18, 2020 22:57
@mmmavis mmmavis merged commit 48fa51a into master Mar 19, 2020
@mmmavis mmmavis deleted the issue-4287-html-p branch March 19, 2020 17:18
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.

Highlight component's decription should be wrapped in <p></p>
3 participants