distance_of_time_in_words returns a string instead of text element

[edwardloveall]

Purpose

closes #780

Description

Returning a text element (which uses the IO class) can cause weird bugs, like the entire HTML source of the page being printed out into the page. This alters the distance_of_time_in_words method to return a string instead which can be inserted into a text element, as an attribute of a different element, or even outside of a view entirely!

Checklist

• - An issue already exists detailing the issue/or feature request that this PR fixes
• - All specs are formatted with crystal tool format spec src
• - Inline documentation has been added and/or updated
• - Lucky builds on docker with ./script/setup
• - All builds and specs pass on docker with ./script/test

[paulcsmith]

I think before merging this in wed also want to change a bunch of the other helpers methods that write directly to the IO. Otherwise some will work differently than others. Stuff like pluralize https://github.com/luckyframework/lucky/blob/master/src/lucky/page_helpers/text_helpers.cr

Some use raw so it doesn’t make sense to convert. Those should be documented that they write directly to IO though and should probably use a Nil return type so people don’t try to use them

[edwardloveall]

Yeah, makes sense. I'll do that in a bit 👍

[paulcsmith]

Nice branch name 😃

[jwoertink]

0.14 has already been released, so this would more likely go under ### Changes since v0.14. However, with that said, maybe all changelog changes can just go in a single commit? Kind of a pain to update all at once, but it helps to know what docs will need to be updated and such..

[edwardloveall]

I made some progress on this. However, I'm not sure how to approach truncate because of its block overload. I'm not sure how to get it to run the block and then safely add the result onto the end of the truncated string. Here's my test case from crystal play:

require "./src/lucky"
require "./spec/support/context_helper"
include ContextHelper

class TextHelperTestPage
  include Lucky::HTMLPage

  def render
  end
end

view = TextHelperTestPage.new(build_context)

view.link "#" do
  text = view.truncate("Here is a long test and I need a continue to read link", length: 27) do
    view.span "Continue"
  end.to_s
  view.text text
end.to_s

This returns some extra HTML because it's grabbing the existing HTML in the truncate method. I'd love an assist.

[paulcsmith]

I think that is a pretty tricky edge case. Maybe for now we make the block version a different method. Like raw_truncate or something. What do you think?

[edwardloveall]

Yeah I like that. There's a private method called truncate_text that I think will do what we want. I'll make that public and write some docs.

[edwardloveall]

Cool, should be ready to go unless you want me to move the changelog stuff.

[paulcsmith]

2 small things actually :D then ready to go!

[paulcsmith]

Actually, could you make the return types : Nil for any of the methods that are meant to write HTML directly to the IO so that it won't ever accidentally be used as a String? Might also be worth adding a note like: This method writes HTML directly to the page. It does not return a String or something like that. Thoughts?

[paulcsmith]

Might be worth putting in bold **Breaking change** - Many text... since this will definitely break stuff using it

[edwardloveall]

@paulcsmith done in 537ee85

[paulcsmith]

Perfect! Thank you