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

Expose output helpers to ApplicationComponent #168

Closed
bradgessler opened this issue Mar 18, 2024 · 1 comment · Fixed by #169
Closed

Expose output helpers to ApplicationComponent #168

bradgessler opened this issue Mar 18, 2024 · 1 comment · Fixed by #169
Milestone

Comments

@bradgessler
Copy link
Contributor

To adapt a Rails helper today, developers have to follow the pattern of creating an adapter that's established at https://github.com//phlex-ruby/phlex-rails/blob/version-1-9/lib/phlex/rails/helpers.rb

The define.*_output_helper methods could be exposed directly to ApplicationComponent as class methods to make adapting to third party helpers, like local_time, slightly easier without having to implement a module, like this:

class ApplicationComponent < Phlex::HTML
    include Phlex::Rails::Helpers::Routes
    include Phlex::Rails::HelperMacros

    define_output_helper :local_time

    # ...

The method names may need to be tweaked slightly, like define_output_helper becomes output_helper :local_time or delegate_to_output_helper :local_time

@joeldrapper
Copy link
Collaborator

joeldrapper commented Mar 18, 2024

I think we could go further and include them in a patch to Phlex::HTML directly. We already have register_element.

We can simplify things by only providing output_helper_with_capture_block and value_helper_with_capture_block, since those two work with or without blocks, and the 0.1% performance gain from not taking a block isn’t worth it here in my opinion.

We should probably call them register_value_helper and register_output_helper, though I’m open to suggestions for the names.

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 a pull request may close this issue.

2 participants