- Read Tutorial
In this lesson we're going to implement the styles for the Topic page. If you look through the templates you may notice something a little odd, there are two templates that could be considered the show page:
app/views/topics/show.html.erbapp/views/topics/posts/index.html.erb
This overlap is due to the nested setup we have between topics and posts. It's important that we don't have any orphaned code in the project. What is orphan code you ask? Orphan code is any code in a project that is not utilized at all. So if we have two pages, such as we have above for topics that should serve the same purpose, one of them needs to be removed in order to clean up the project codebase. I can tell you from experience how frustrating it is to take over the work on a legacy application and work on a file only to find out that it serves zero purpose.
Before we start going Rambo on the Topic show page, let's put together an organized strategy, the steps to take will be:
Decide which page should be removed
Review the specs to see how they need to be refactored and fix any failing tests
Remove the unnecessary routes
Remove the unnecessary methods
Finally, remove the orphaned file
Now that we know what we need to do, let's get to refactoring!
Decide which page should be removed
Starting up the rails server and navigating to both pages we'll see that the Topic show page by itself doesn't really do anything, whereas the post index page displays the posts nested under the topic. based on the fact that the functionality is already there it makes the most sense to take advantage of the nested structure and have the index page to essentially double as the Topic show page.
Review the specs to see how they need to be refactored and fix any failing tests
Now that we know what we're going to do, let's look at the specs that will need to be refactored. In the spec/features/topic_spec.rb file you'll see that we have a number of tests that are pointing to the page that we'll be removing. Let's refactor these specs so they point to the posts index page. Thankfully because we're using a before action we'll be able to make this change with minimal work (which is typically a good sign that we're building the application properly). In fact it looks like we only need to make two code changes to get this updated in the spec, I've placed them below:
# spec/features/topic_spec.rb it 'each topic links to its show page' do visit topics_path expect(page).to have_link(@topic.title, href: topic_posts_path(topic_id: @topic)) end before do visit topic_posts_path(topic_id: @topic) end
These changes update the test expectations to point to the posts index action instead of the topic show page. If you run the specs you'll see that only two tests are failing:
Failures:
1) navigate index each topic links to its show page
Failure/Error: expect(page).to have_link(@topic.title, href: topic_posts_path(topic_id: @topic))
expected to find link "Sports" with href "/topics/sports/posts" but there were no matches. Also found "Sports", which matched the selector but not all filters.
# ./spec/features/topic_spec.rb:22:in `block (3 levels) in <top (required)>'
2) navigate show should display the topic title
Failure/Error: expect(page).to have_css('h1', text: 'Sports')
expected to find css "h1" with text "Sports" but there were no matches
# ./spec/features/topic_spec.rb:36:in `block (3 levels) in <top (required)>'
Finished in 1.55 seconds (files took 2.55 seconds to load)
35 examples, 2 failures, 2 pending
Failed examples:
rspec ./spec/features/topic_spec.rb:20 # navigate index each topic links to its show page
rspec ./spec/features/topic_spec.rb:35 # navigate show should display the topic title
These lead us to see that we need to make two code changes in the implementation code:
Update the link on the
Topicindexpage to point to the correct pageAdd an
h1header element on theindexpage (which is something we were going to do anyways to match the mock
Let's start with the link on the Topic index page, update the partial to point to the correct page:
<!-- app/views/topics/_topic.html.erb --> <div> <%= link_to topic.title, topic_posts_path(topic_id: topic) %> </div>
If you run the tests now you'll see that the change fixed the first failure and to fix the second we'll implement the smallest change possible on the posts index page to get the test passing since we're going to be implementing the design shortly. Below is what the updated file should look like:
<!-- app/views/topics/posts/index.html.erb --> <h1><%= @topic.title %></h1> <% @posts.each do |post| %> <%= link_to post.title, topic_post_path(topic_id: @topic, id: post) %> <%= post.content %> <%= post.user.username %> <% end %> <%= link_to "New Post", new_post_path(topic_id: @topic.id) %>
Running rspec for the full project will now show that the tests are all passing, nice work!
Remove the unnecessary routes
In the same way where I don't like having orphaned files, I also don't like having orphaned routes. With the resources route helper method in Rails it's very easy to forget that it creates 7 different routes for us and we need to explicitly define which ones we don't need. Open up the routes file and update the following line:
# config/routes.rb resources :topics, except: [:show] do
The resources route helper method takes in arguments, where you can list any routes that you want to exclude. If you run rspec you'll see that even though we refactored the specs that used the old show page the forms are redirecting to the page, so we need to update the controller create and update method, as shown below:
# app/controllers/topics_controller.rb def create @topic = Topic.new(topic_params) if @topic.save redirect_to topic_posts_path(topic_id: @topic), notice: 'Topic was successfully created.' else render :new end end def update if @topic.update(topic_params) redirect_to topic_posts_path(topic_id: @topic), notice: 'Your topic was successfully updated.' else render :edit, notice: 'There was an error processing your request!' end end
Now if you run rspec you'll see that the tests are all back to passing and our routes have been cleaned up.
Remove the unnecessary methods
This is an easy one, simply delete the show method from the topics_controller.rb file. Run the tests when you've done that and you'll see that all the specs are still passing.
Finally, remove the orphaned file
This is another easy one, you can use the following command in the terminal or delete the file directly in the text editor rm app/views/topics/show.html.erb.
Now our application is working and our Topic page is cleaned up. With that all in place we can integrate the design. Here is the proposed design for the Topic show page from the UI/UX guide:
The designer didn't quite understand the requirements, so this page isn't going to work for our needs, the UI definitely isn't what a modern application will need. This is something that happens with regularity when building out applications and working with teams and designers (which is part of why I included it even after reviewing it when it was sent over).
So what are we going to do? Well, if you look at the search page, you may notice that this is much closer to the functionality that we're looking for:
This looks more like what you'd expect from a production application from a user interface perspective. With a few small changes this will be perfect. Let's pull in the post list code from the search page and place it in the posts/index template. We don't need the weird navigation items or the widgets, those would just clutter up the page.
<!-- app/views/topics/posts/index.html.erb --> <div class="container"> <div class="row"> <div class="col-xs-9 pull-left"> <ul class="articles"> <li class="clearfix"> <div class="ratings"> <a href="#up" class="up"></a> <h1>1178</h1> <a href="#down" class="down"></a> </div> <div class="article-image"> <div class="mask"></div> <img src="images/common/article_image.png"/> </div> <div class="article-desc"> <h2>How outer space is becoming the...</h2> <p>Duis aute irure dolor in reprehenderit nulla <strong>deserunt mollit</strong> anim id...</p> <span>By <strong>Thomas Gibbons-Neff</strong> June 20 at 4:05 PM</span> </div> <div class="article-stats"> <ul class="pull-right"> <li> <span class="icon icon-view"></span> <h2>347</h2> </li> <li> <span class="icon icon-comment"></span> <h2>347</h2> </li> <li> <span class="icon icon-connection"></span> <h2>347</h2> </li> </ul> </div> </li> </ul> </div> </div> </div>
This will give us the post element and nothing else, now we simply have to replace the values with the dynamic ones passed in from the controller.
<!-- app/views/topics/posts/index.html.erb --> <div class="container"> <h1><%= @topic.title %></h1> <div class="row"> <div class="col-xs-9 pull-left"> <ul class="articles"> <% @posts.each do |post| %> <li class="clearfix"> <div class="ratings"> <a href="#up" class="up"></a> <h1>1178</h1> <a href="#down" class="down"></a> </div> <div class="article-image"> <div class="mask"></div> <img src="images/common/article_image.png"/> </div> <div class="article-desc"> <h2><%= link_to post.title, topic_post_path(topic_id: @topic, id: post) %></h2> <p><%= post.content %></p> <span>By <strong><%= post.user.username %></strong> <%= post.created_at %></span> </div> <div class="article-stats"> <ul class="pull-right"> <li> <span class="icon icon-view"></span> <h2>347</h2> </li> <li> <span class="icon icon-comment"></span> <h2>347</h2> </li> <li> <span class="icon icon-connection"></span> <h2>347</h2> </li> </ul> </div> </li> <% end %> </ul> </div> </div> <%= link_to "New Post", new_post_path(topic_id: @topic.id) %> </div>
If you refresh the page now you'll see that that worked and now our Topic page is now looking much better:
Obviously we have a few asset issues since the image on the left hand side isn't working and none of the icons are showing up, however the page structure itself is shaping up nicely. In the next lesson we'll get the icons working.