Merge pull request #8 from jdickey/styling

Styling cleanup completed per Issues #5, #6, and #7.
jdickey · Dec 28, 2015 · e9e504a · e9e504a
2 parents c4ce908 + 7ce7b7d
commit e9e504a
Show file tree

Hide file tree

Showing 13 changed files with 285 additions and 119 deletions.
diff --git a/Gemfile b/Gemfile
@@ -76,6 +76,7 @@ group :test do
   gem 'minitest-matchers', '~> 1.4'
   gem 'minitest-reporters', '~> 1.0'
   gem 'minitest-fail-fast', '~> 0.1'
+  gem 'minitest-tagz', '~> 1.2'
   gem 'selenium-webdriver', '~> 2.47'
   # gem 'capybara-webkit', '~> 1.6'
   gem 'poltergeist', '~> 1.6'

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -193,6 +193,10 @@ GEM
       builder
       minitest (>= 5.0)
       ruby-progressbar
+    minitest-tagz (1.2.1)
+      minitest (~> 5)
+      shoulda-context
+      state_machine
     multi_json (1.11.2)
     netrc (0.11.0)
     nokogiri (1.6.7.1)
@@ -298,6 +302,7 @@ GEM
       railties (>= 3.2.0)
       sequel (>= 3.28, < 5.0)
     sexp_processor (4.6.0)
+    shoulda-context (1.2.1)
     simplecov (0.11.1)
       docile (~> 1.1.0)
       json (~> 1.8)
@@ -311,6 +316,7 @@ GEM
       actionpack (>= 4.0)
       activesupport (>= 4.0)
       sprockets (>= 3.0.0)
+    state_machine (1.2.0)
     term-ansicolor (1.3.2)
       tins (~> 1.0)
     thin (1.6.4)
@@ -379,6 +385,7 @@ DEPENDENCIES
   minitest-matchers (~> 1.4)
   minitest-rails-capybara (~> 2.1)
   minitest-reporters (~> 1.0)
+  minitest-tagz (~> 1.2)
   ox (~> 2.2)
   parser (~> 2.2)
   pg (~> 0.18)

diff --git a/Rakefile b/Rakefile
@@ -44,7 +44,7 @@ if %w(development test).include? Rails.env
 
   FlogTask.new do |t|
     t.verbose = true
-    t.threshold = 300 # default is 200
+    t.threshold = 400 # default is 200
     t.instance_variable_set :@methods_only, true
     t.dirs = %w(app lib) # Look, Ma! No tests! (Run for `test` periodically.)
   end

diff --git a/app/assets/stylesheets/things.scss b/app/assets/stylesheets/things.scss
@@ -1,3 +1,29 @@
 // Place all the styles related to the Things controller here.
 // They will automatically be included in application.css.
 // You can use Sass (SCSS) here: http://sass-lang.com/
+
+$form-error-colour: #a94442;
+$shadow-alpha: .75;
+$shadow-width: 2px;
+
+table.table {
+    margin-top: 20px;
+}
+
+button.btn-primary {
+    margin-top: 12px;
+}
+
+// This, and the variable definitions it uses (above), are the fix for
+// Issue #7.
+div.field_with_errors {
+  input{
+    border-color: $form-error-colour;
+    -webkit-box-shadow: inset 0 $shadow-width $shadow-width rgba(0, 0, 0, $shadow-alpha);
+            box-shadow: inset 0 $shadow-width $shadow-width rgba(0, 0, 0, $shadow-alpha);
+  }
+
+  label {
+    color: $form-error-colour;
+  }
+}
diff --git a/app/controllers/things_controller/create_responder.rb b/app/controllers/things_controller/create_responder.rb
@@ -5,7 +5,7 @@
 class ThingsController < ApplicationController
   # Encapsulates all logic involved in create action, successful or otherwise.
   class CreateResponder
-    # include SemanticLogger::Loggable
+    include SemanticLogger::Loggable
 
     def initialize(controller)
       @controller = controller
@@ -30,6 +30,8 @@ def create_thing(params)
     def create_and_save_thing(params)
       create_thing params
       @thing.save if @thing.valid?
+      # Rails.logger.debug 'Error messages for @thing',
+      #                    @thing.errors.full_messages
       self
     end
 
@@ -40,6 +42,7 @@ def handle_result
 
     def handle_failure
       # Controller's @thing should already be set by this point per yield
+      # Rails.logger.debug 'Flashes in responder', flash: controller.flash.to_h
       controller.render 'new'
       self
     end

diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
@@ -1,15 +1,26 @@
 <!DOCTYPE html>
 <html>
 <head>
+  <meta charset="utf-8">
+  <meta http-equiv="X-UA-Compatible" content="IE=edge">
+  <meta name="viewport" content="width=device-width, initial-scale=1">
   <% @title_text = content_for?(:title) ? yield(:title) : 'Generic Title?' %>
   <title><%= @title_text %></title>
   <%= stylesheet_link_tag    'application', media: 'all' %>
+  <%= stylesheet_link_tag '//maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css' %>
   <%= javascript_include_tag 'application' %>
+  <%= javascript_include_tag  '//maxcdn.bootstrapcdn.com/bootstrap/3.3.6/js/bootstrap.min.js' %>
   <%= csrf_meta_tags %>
 </head>
 <body>
 
-<%= yield %>
+<div class="container">
+  <div class="row">
+    <div class="col-lg-12 col-md-12 col-sm-12 col-xs-12">
+      <%= yield %>
+    </div>
+  </div>
+</div>
 
 </body>
 </html>
diff --git a/app/views/things/index.rb b/app/views/things/index.rb
@@ -5,29 +5,39 @@
 class Views::Things::Index < Views::Base
   needs :things, flash: {}
 
-  include Views::Things::Shared
-
   def content
-    widget Flashes, flashes: flash
+    widget Views::Things::Shared::Flashes, flashes: flash
     non_detail_widgets
     detail_table
   end
 
   private
 
   def add_thing_button
-    widget AddThingButton
+    row_with_single_column { widget AddThingButton }
   end
 
   def detail_table
-    table_widget { detail_lines }
+    row_with_single_column { table_widget { detail_lines } }
   end
 
   def non_detail_widgets
     messages = [:title_content, :page_header, :add_thing_button]
     messages.each { |message| send message }
   end
 
+  def row_with_single_column
+    styled_row { single_column { yield } }
+  end
+
+  def single_column
+    div(class: 'col-md-12') { yield }
+  end
+
+  def styled_row
+    div(class: 'row') { yield }
+  end
+
   def title_content
     content_for :title, 'Model Form Demo'
   end

diff --git a/app/views/things/index/table_widget.rb b/app/views/things/index/table_widget.rb
@@ -11,14 +11,18 @@ class TableWidget < Views::Base
     needs :things, detail_fields: Thing.new.columns
 
     def content
-      table do
+      containing_table do
         header_row_widget
         render_detail_lines
       end
     end
 
     private
 
+    def containing_table
+      table(class: 'table table-hover') { yield }
+    end
+
     def header_row_widget
       widget HeaderRow, fields: detail_fields
     end

diff --git a/app/views/things/new.rb b/app/views/things/new.rb
@@ -5,33 +5,30 @@ class Views::Things::New < Views::Base
                 title_content: 'Add New Thing to List of All Things'
 
   include SemanticLogger::Loggable
-  include Views::Things::Shared
 
   def content
     flash_messages
     page_title_content
-    container { form_row }
+    page_header_content
+    form_widget
   end
 
   private
 
-  def container
-    div(class: 'container') { yield }
+  def flash_messages
+    Rails.logger.debug 'Flashes', flash: flash.to_h
+    widget Views::Things::Shared::Flashes, flashes: flash
   end
 
-  def flash_messages
-    widget Flashes, flashes: flash
+  def form_widget
+    widget EntryForm, thing: thing
   end
 
-  def form_row
-    row { widget EntryForm, thing: thing }
+  def page_header_content
+    h1 { text title_content }
   end
 
   def page_title_content
     content_for :title, title_content
   end
-
-  def row
-    div(class: 'row') { yield }
-  end
 end # class Views::Things::New
diff --git a/app/views/things/new/entry_form.rb b/app/views/things/new/entry_form.rb
@@ -3,12 +3,13 @@
 class Views::Things::New < Views::Base
   # Form widget for 'things/new' action.
   class EntryForm < Views::Base
-    needs :thing
+    needs :thing, things_path: '/things', submit_button_text: 'Submit',
+                  form_css_class: 'form-horizontal'
 
     def content
       outer_form do |builder|
         widget OuterFormControls, builder: builder
-        builder.submit 'Submit'
+        builder.submit submit_button_text
       end
     end
 
@@ -19,7 +20,7 @@ def outer_form(&_block)
     end
 
     def outer_form_params
-      { url: '/things', html: { class: 'form-vertical' } }
+      { url: things_path, html: { class: form_css_class } }
     end
   end # class Views::Things::New::EntryForm
 end # class Views::Things::New
diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -65,6 +65,12 @@
 Minitest::Reporters.use! [Minitest::Reporters::DefaultReporter.new(
   color: true, detailed_skip: true, fast_fail: true)]
 
+require 'minitest/tagz'
+tags = ENV['TAGS'].split(',') if ENV['TAGS']
+tags ||= []
+tags << 'focus'
+Minitest::Tagz.choose_tags(*tags, run_all_if_no_match: true)
+
 # Included in generated `test_helper.rb` by Rails' app generator.
 # # Base class for all test cases
 # class ActiveSupport::TestCase

diff --git a/test/views/things/index/table_widget_test.rb b/test/views/things/index/table_widget_test.rb
@@ -22,9 +22,8 @@
     end
 
     describe 'renders an outermost :table element that' do
-      it 'has no CSS styling' do
-        expect(nodes[:class]).must_be :nil?
-        expect(nodes[:style]).must_be :nil?
+      it 'has Bootstrap table CSS styling' do
+        expect(nodes[:class].split).must_equal %w(table table-hover)
       end
 
       it 'contains a :tr element as its first child, containing :th tags' do