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

account for content wrapped in <p> tags & various fixes #11

Merged
merged 1 commit into from
Feb 11, 2014
Merged

account for content wrapped in <p> tags & various fixes #11

merged 1 commit into from
Feb 11, 2014

Conversation

ndimatteo
Copy link
Contributor

I found this plugin incredibly useful, until the content I was trying to shorten was user-generated from a wysiwyg. Most content gets wrapped in <p> tags, so it's not just 'floating.' This caused the plugin to break, as it didn't know how to properly handle them, as they were not tags within the text to be truncated, but rather wrapping the entirety of the content.

This pull request does a few things:

  1. Update the copyright to 2014
  2. use <div> tags for the shortcontent and allcontent wrappers (better semantically, as <p> tags within <span> tags is... weird)
  3. integrates issue Can the HTML tag protection stuff be simplified? #7 that @henrythewasp addressed
  4. removes bottom margin from last <p> tag in shortcontent (so it actually appears cut off)
  5. wraps ellipses text in a <span> with a class of ellip so you can style

If anyone has a better approach to account for <p> tags and other wrapping elements, share em in the comments.

@ndimatteo
Copy link
Contributor Author

I should add, that using <div> tags in place of <span> tags is not just for semantic reasons, it's also because <p> tags are inherently block items, while <span> tags are inline. Using divs allows you to show/hide any block level element without any weirdness. Sort of like a catchall.

viralpatel added a commit that referenced this pull request Feb 11, 2014
account for content wrapped in <p> tags & various fixes
@viralpatel viralpatel merged commit daf9360 into viralpatel:master Feb 11, 2014
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