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

Should text helpers append to the view directly? #349

Closed
tommyschaefer opened this issue Jan 7, 2018 · 1 comment
Closed

Should text helpers append to the view directly? #349

tommyschaefer opened this issue Jan 7, 2018 · 1 comment

Comments

@tommyschaefer
Copy link
Contributor

Hi there!

While working on #348, I started to think that maybe the text helpers should return strings rather than append to the view directly.

In the context of #348, I'm imagining the following scenario:

I have:

class TestPage
  include Lucky::HTMLPage

  needs list : Array

  def render
    to_string(list)
  end
end

which renders when given a list %(a b c):

a, b, and c

Now, I would like to render I have the following items: a, b, and c

To me, I think it feels a bit more natural to write:

text "I have the following items: #{to_sentence(list)}"

over:

text "I have the following items: "
to_sentence(list)

I also wonder if appending to the view is maybe a bit too much for a text helper to do? To me, it feels like that maybe falls outside of the helpers responsibility? How do you all feel about this?

Thank you so much for your time! Like I said in one of my other issues, I'm really enjoying Lucky and have a tremendous appreciation for all of the work that has been put in to it! 😄

@jwoertink
Copy link
Member

Now that #781 has been merged in, the next version will have this change. Text helpers will return a string instead of rendering directly to view.

djuber pushed a commit to djuber/lucky that referenced this issue Aug 29, 2021
…yframework#349)

* Rename Form -> SaveOperation

Part of luckyframework/avram#104

* Update the app skeleton to use 'permit_columns' instead of 'fillable'

* Update the app skeleton to use 'Avram::PermittedAttribute'

instead of 'Avram::FillableField'
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

No branches or pull requests

3 participants