komposable / komponent

An opinionated way of organizing front-end code in Ruby on Rails, based on components
http://komponent.io
MIT License
426 stars 31 forks source link

Make application routes usable on example views #141

Open nicolas-brousse opened 5 years ago

nicolas-brousse commented 5 years ago

If we want to have a component that works directly with a model object, Rails application returns an error.

With the following example.

# frontent/components/post_card/_examples.html.erb
<%= cdoc "post_card", post: Post.first %>
# frontent/components/post_card/_post_card.html.erb
<div class="post_card">
<%= link_to Post.name, post_path(post) %>
</div>

We have the following error:

undefined method `post_path' for #<#<Class:0x00007f579c3f6c90>:0x00007f579c36b668>

Explanation

The komponent gem is isolated so application routes are not available from Komponent::StyleguideController.

Proposal 1

The changes I did expose Rails application routes in Komponent::StyleguideController. And I named the engine to be able to access komponent application routes.

Proposal 2 (current code state of this PR)

Stop to isolate the engine. So routes are loaded in application. But helper will be loaded too.

nicolas-brousse commented 5 years ago

I'm currently reading an article and some documentation to be sure of my proposal.

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 815


Totals Coverage Status
Change from base Build 814: 0.003%
Covered Lines: 181
Relevant Lines: 182

💛 - Coveralls
nicolas-brousse commented 5 years ago

Proposal 2

diff --git a/config/routes.rb b/config/routes.rb
index 7293246..cfa10cc 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -1,5 +1,5 @@
 # frozen_string_literal: true

 Komponent::Engine.routes.draw do
-  resources :styleguide, only: %i[index show]
+  resources :styleguide, only: %i[index show], module: :komponent
 end
diff --git a/lib/generators/komponent/templates/styleguide/components/sidebar/_komponent_sidebar.html.erb b/lib/generators/komponent/templates/styleguide/components/sidebar/_komponent_sidebar.html.erb
index 195ade3..f6198a5 100644
--- a/lib/generators/komponent/templates/styleguide/components/sidebar/_komponent_sidebar.html.erb
+++ b/lib/generators/komponent/templates/styleguide/components/sidebar/_komponent_sidebar.html.erb
@@ -3,7 +3,7 @@
   <ul class="komponent-sidebar-items">
     <%- components.each do |_k, component| %>
       <li class="komponent-sidebar-item">
-        <%= link_to_unless_current component.title, styleguide_path(id: component.id) %>
+        <%= link_to_unless_current component.title, komponent.styleguide_path(id: component.id) %>
       </li>
     <% end %>
   </ul>
diff --git a/lib/komponent/engine.rb b/lib/komponent/engine.rb
index 477160c..41c40da 100644
--- a/lib/komponent/engine.rb
+++ b/lib/komponent/engine.rb
@@ -9,7 +9,7 @@ require 'komponent/translation'

 module Komponent
   class Engine < Rails::Engine
-    isolate_namespace Komponent
+    engine_name "komponent"

     rake_tasks do
       load 'komponent/rails/tasks/komponent.rake'
nicolas-brousse commented 5 years ago

Since we don't have helper in app/helpers, I prefer proposal 2. Sounds more clean to me.

I don't think we really needs to isolate our engine.

Spone commented 5 years ago

Since we don't have helper in app/helpers, I prefer proposal 2. Sounds more clean to me.

I don't think we really needs to isolate our engine.

I agree. Can you implement solution #2 in your PR?

nicolas-brousse commented 5 years ago

I'll try to found some time in the next days to rework on this.

nicolas-brousse commented 4 years ago

I updated the code to implement proposal 2

Spone commented 4 years ago

NB: We will need to inform users that they need to replace styleguide_path by komponent.styleguide_path in styleguide/components/sidebar

nicolas-brousse commented 4 years ago

Done