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

Strip html from the event description #53

Merged
merged 4 commits into from Aug 25, 2018

Conversation

drewvolz
Copy link
Member

@drewvolz drewvolz commented Aug 11, 2018

Closes #52
Closes #32

@drewvolz drewvolz requested review from hawkrives and rye August 11, 2018 04:47
rye
rye previously approved these changes Aug 11, 2018
@drewvolz
Copy link
Member Author

It strips html, but doesn't add any spaces. It works well if surrounding tags are within the content, but looks a little interesting when links are appended to the end of a listing and end up connected to a period from the sentence before. I do not know how or if we want to tackle that.


function convertGoogleEvents(data, now = moment()) {
let events = data.map(event => {
const startTime = moment(event.start.date || event.start.dateTime)
const endTime = moment(event.end.date || event.end.dateTime)

const description = JSDOM.fragment(event.description).textContent.trim()
Copy link
Member

Choose a reason for hiding this comment

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

Add a blank line after this, please.

@hawkrives
Copy link
Member

I think .innerText will preserve linebreaks (
https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent#Differences_from_innerText) - try it out?

@drewvolz
Copy link
Member Author

@hawkrives I don't think it's supported in jsdom jsdom/jsdom#1245

@hawkrives
Copy link
Member

Darn. We could just replace <br> tags with newlines… ?

@drewvolz
Copy link
Member Author

Well, the issue I see is when we have something like a link tag. You can strip all the html, but then it also removes separating white space. You end up with text that has a url concatenated with the previous word or punctuation.

@hawkrives
Copy link
Member

If you replace the br tag with a space or a newline?

@drewvolz
Copy link
Member Author

Not everything with html tags will contain a newline, is my point?

@hawkrives
Copy link
Member

Yeah, it won't solve everything, but textContent will preserve in-line spaces (ie "some text (link)" will still have the space between text and link), and if we replace p-closing and br tags with \n, that should get us most of the way there?

@drewvolz
Copy link
Member Author

I guess.


function convertGoogleEvents(data, now = moment()) {
let events = data.map(event => {
const startTime = moment(event.start.date || event.start.dateTime)
const endTime = moment(event.end.date || event.end.dateTime)
const description = JSDOM.fragment(event.description).textContent.trim()
Copy link
Member

Choose a reason for hiding this comment

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

Let's do the simple thing first, and just change this to

let description = event.description.replace('<br>', '\n')
description = JSDOM.fragment(description).textContent.trim()

so that we at least catch the <br> tags.

@hawkrives hawkrives force-pushed the remove-event-description-html branch from 7067808 to e90b6ce Compare August 25, 2018 14:54
@hawkrives hawkrives merged commit 5de1915 into master Aug 25, 2018
@rye rye deleted the remove-event-description-html branch September 4, 2018 16:55
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.

None yet

3 participants