Merge pull request #1225 from ariana-paris/Issue_3197-User_skin_UI

Issue 3197 user skin ui

app/controllers/skins_controller.rb

@@ -86,7 +86,7 @@ def index
         else
           @skins = Skin.approved_skins.usable.site_skins.cached.sort_by_recent_featured
         end
-        @title = ts('Public Skins')
+        @title = ts('Public Site Skins')
       end
     end
   end

app/views/skins/_form.html.erb

@@ -1,14 +1,14 @@
 <fieldset>
   <legend>About</legend>
-  <h3 class="landmark heading">About</h3>
+  <h3 class="landmark heading">ts('About')</h3>
   <dl>      
-    <dt><%= label_tag "skin_type", ts('Type') %> <%= link_to_help "skins-basics" %></dt>
+    <dt><%= label_tag 'skin_type', ts('Type') %> <%= link_to_help 'skins-basics' %></dt>
     <% if @skin.id %>
       <% # don't let user change the type of an already-created skin %>
-      <dd><%= (@skin.type && @skin.type == 'WorkSkin') ? ts("Work Skin") : ts("Site Skin") %></dd>
+      <dd><%= (@skin.type && @skin.type == 'WorkSkin') ? ts('Work Skin') : ts('Site Skin') %></dd>
     <% else %>      
       <dd>
-        <%= select_tag "skin_type", options_for_select(Skin::TYPE_OPTIONS, @skin.type || params[:skin_type] ), 
+        <%= select_tag 'skin_type', options_for_select(Skin::TYPE_OPTIONS, @skin.type || params[:skin_type] ),
           :onchange => 'if ($j(this).val() == "WorkSkin") {$j(".verbose").hide();} else {$j(".verbose").show();}' %>
       </dd>
     <% end %>
@@ -19,33 +19,33 @@
     <dt><%= f.label :description, ts('Description') %></dt>
     <dd><%= f.text_field :description %></dd>
 
-    <dt><%= f.label :icon, ts("Upload a preview (png, jpeg or gif)") %></dt>
+    <dt><%= f.label :icon, ts('Upload a preview (png, jpeg or gif)') %></dt>
     <dd><%= f.file_field :icon %></dd>
 
-    <dt><%= f.label :public, ts('Apply to make public') %> <%= link_to_help "skins-approval" %></dt>
+    <dt><%= f.label :public, ts('Apply to make public') %> <%= link_to_help 'skins-approval' %></dt>
     <dd><%= f.check_box :public %></dd>
   </dl>
 </fieldset>
 
 <fieldset>
   <legend>CSS</legend>
-  <h3 class="heading"><%= f.label :css, ts('CSS') %> <%= link_to_help "skins-creating" %></h3>
-    <p><%= f.text_area :css, :cols => 70, :class => "large" %></p>        
+  <h3 class="heading"><%= f.label :css, ts('CSS') %> <%= link_to_help 'skins-creating' %></h3>
+    <p><%= f.text_area :css, :cols => 70, :class => 'large' %></p>
 </fieldset>
 
 <fieldset class="verbose<%= (@skin.type || params[:skin_type]) == 'WorkSkin' ? ' hidden' : ''%>">
   <legend>Advanced 
   <span class="action">
-    <a href="#" class="advanced_skin_open hidden"><%= ts("Show") %> &#8595;</a>
-    <a href="#" class="advanced_skin_close hidden"><%= ts("Hide") %> &#8593;</a>
+    <a href="#" class="advanced_skin_open hidden"><%= ts('Show') %> &#8595;</a>
+    <a href="#" class="advanced_skin_close hidden"><%= ts('Hide') %> &#8593;</a>
   </span>
   </legend>
 
 
   <div id="advanced_skin" class="<%= show_advanced_skin?(@skin) ? 'toggled open' : 'toggled' %>">
     <fieldset>
-      <legend>Conditions <%= link_to_help "skins-conditions" %></legend>
-      <h4 class="heading landmark">Conditions</h4>
+      <legend><%= ts('Conditions') %> <%= link_to_help 'skins-conditions' %></legend>
+      <h4 class="heading landmark"><%= ts('Conditions') %></h4>
       <dl>
         <dt><%= f.label :role, ts('What it does: ')%></dt>
         <dd>
@@ -56,37 +56,37 @@
         <dt><%= f.label :ie_condition, ts('IE Only: ')%></dt>
         <dd><%= f.select :ie_condition, options_for_select(Skin::IE_CONDITIONS, @skin.ie_condition), :include_blank => true %></dd>
 
-        <dt><%= f.label :unusable, ts("Parent Only: ")%></dt>
+        <dt><%= f.label :unusable, ts('Parent Only: ')%></dt>
         <dd><%= f.check_box :unusable %></dd>
 
         <dt><%= f.label :media, ts('Media: ')%></dt>
         <dd>
           <fieldset class="listbox group">
-          <h5 class="heading">Choose @media</h5>
+          <h5 class="heading"><%= ts('Choose media') %></h5>
             <%= checkbox_section(f, :media, Skin::MEDIA, :checked_method => @skin.media || Skin::DEFAULT_MEDIA, :value_method => "to_s", :name_method => "to_s", :include_blank => false) %>
           </fieldset>
         </dd>        
       </dl>
     </fieldset>
 
     <fieldset class="listbox group">
-      <legend>Parent Skins <%= link_to_help "skins-parents" %></legend>
-      <h4 class="heading landmark">Parent Skins</h4>
+      <legend><%= ts('Parent Skins') %> <%= link_to_help 'skins-parents' %></legend>
+      <h4 class="heading landmark"><%= ts('Parent Skins') %></h4>
        <ul class="index group">
        <% f.object.skin_parents.each_with_index do |parent, index| %>
         <%= f.fields_for :skin_parents, parent do |parent_form| %>
-         <%= render "skin_parent_fields", :form => parent_form, :index => index %>
+         <%= render 'skin_parent_fields', :form => parent_form, :index => index %>
         <% end %>
       <% end %>
 
       <% if f.object.skin_parents.count == 0 %><li class="last_id" style="display:none;">0</li><% end %>
-      <li class="actions" role="button"><%= link_to_add_section(ts("Add parent skin"), f, :skin_parents, "skin_parent_fields") %></p>
+      <li class="actions" role="button"><%= link_to_add_section(ts('Add parent skin'), f, :skin_parents, "skin_parent_fields") %></li>
 
-      <li class="archive_parents<%= (f.object.role == "override") ? '' : ' hideme' %>">
+      <li class="archive_parents<%= (f.object.role == 'override') ? '' : ' hideme' %>">
         <p>
           <label for="add_site_parents">
-            <%= ts("Load Archive Skin Components: ") %><%= link_to_help "skins-parents" %>
-            <%= check_box_tag "add_site_parents" %>
+            <%= ts('Load Archive Skin Components: ') %><%= link_to_help 'skins-parents' %>
+            <%= check_box_tag 'add_site_parents' %>
           </label>
         </p>
       </li>

app/views/skins/_skin_parent_fields.html.erb

@@ -1,10 +1,11 @@
 <% index ||= 1 %>
 <li class="removeme">
-    <fieldset>
-    <p><span class="actions cancel" title="cancel"><%= link_to_remove_section ts("x"), form %></span>
-    <%= ts("Parent #") %><%= form.text_field :position, :class => "number", :value => (form.object.position || index), :title => "position of parent as integer" %>
-      </p>
-        <%= form.text_field :parent_skin_title, autocomplete_options("site_skins", :autocomplete_token_limit => 1, :title => "title of parent skin") %>
-      </fieldset>
+  <fieldset>
+    <p>
+      <span class="actions cancel" title="<%= ts('cancel')%>"><%= link_to_remove_section ts('x'), form %></span>
+      <%= ts('Parent #') %><%= form.text_field :position, :class => 'number', :value => (form.object.position || index), :title => ts('position of parent as integer') %>
+    </p>
+    <%= form.text_field :parent_skin_title, autocomplete_options('site_skins', :autocomplete_token_limit => 1, :title => ts('title of parent skin')) %>
+  </fieldset>
 </li>
-<li class="last_id" style="display:none;"><%= index.is_a?(String) ? index : index+1 %></li>
+<li class="last_id" style="display:none;"><%= index.is_a?(String) ? index : index + 1 %></li>

app/views/skins/_skin_top_navigation.html.erb

@@ -1,36 +1,28 @@
-<% work_skin = params[:skin_type] && params[:skin_type] == "WorkSkin" %>
+<% work_skin = params[:skin_type] && params[:skin_type] == 'WorkSkin'
+   revert_default = ts('Revert to Default Skin') %>
 
 <!--subnav-->
 <h3 class="landmark heading">Navigation</h3>
 <ul class="navigation actions" role="navigation">
-	<li>
-    <% if work_skin %>
-      <%= link_to ts("Site Skins"), url_for(:skin_type => "Skin") %>
-    <% else %>
-      <%= link_to ts("Work Skins"), url_for(:skin_type => "WorkSkin") %>
-    <% end %>
-  </li>
-  <li><%= span_if_current ts('Public Skins'), skins_path %></li>
   <% if session[:site_skin] %>
-    <li><%= link_to ts("Remove Skin"), unset_skins_path %></li>
+    <li><%= link_to revert_default, unset_skins_path %></li>
   <% end %>
 	<% if logged_in? %>
-	  <% if current_user.preference.skin_id && session[:site_skin].blank? %>
+	  <% if (current_user.preference.skin_id != Skin.default.id) && session[:site_skin].blank? %>
 	    <li>
-    	  <%= form_for(current_user.preference, :url => {:controller => :preferences, :action => :update, :user_id => current_user}, :method => "put") do |f| %>
+    	  <%= form_for(current_user.preference, :url => {:controller => :preferences, :action => :update, :user_id => current_user}, :method => 'put') do |f| %>
           <%= f.hidden_field :skin_id, :value => Skin.default.id %>
-          <%= f.submit ts("Remove Skin") %>
+          <%= f.submit revert_default %>
         <% end %>
       </li>
     <% end %>
-
-    <li><%= span_if_current ts('My Skins'), user_skins_path(current_user) %></li>
-    <% unless controller.action_name == "new" %>
-      <li><%= link_to ts('Create Skin'), new_skin_path(:skin_type => work_skin ? "WorkSkin" : "Skin") %></li>
+
+    <% unless controller.action_name == 'new' %>
+      <li><%= link_to ts('Create New Skin'), new_skin_path(:skin_type => work_skin ? "WorkSkin" : "Skin") %></li>
     <% end %>
 
     <!-- wizard-to-custom-css switchers for new and edit -->
-    <% if controller.action_name == "new" ||  (controller.action_name == "edit" && !@skin.is_a?(WorkSkin)) %>
+    <% if controller.action_name == 'new' ||  (controller.action_name == 'edit' && !@skin.is_a?(WorkSkin)) %>
       <% if params[:wizard] %>
         <li id="form-link">
            <%= link_to( ts('Write Custom CSS Instead?'),

app/views/skins/_skin_type_navigation.html.erb

@@ -0,0 +1,24 @@
+<!--subnav-->
+<h3 class="landmark heading"><%= ts('Skin Type Navigation') %></h3>
+
+  <ul class="navigation actions" role="navigation">
+  <% if logged_in? %>
+    <li>
+      <%= span_if_current ts('My Site Skins'), user_skins_path(current_user, :skin_type => 'Site'), current_page?(user_skins_path(current_user)) && (params[:skin_type].blank? || params[:skin_type] == 'Site')  %>
+    </li>
+    <li>
+      <%= span_if_current ts('My Work Skins'), user_skins_path(current_user, :skin_type => 'WorkSkin') %>
+    </li>
+  <% end %>
+  <% unless action_name == 'new' %>
+    <li>
+      <%= span_if_current ts('Public Site Skins'), skins_path(:skin_type => 'Site'), current_page?(skins_path) && (params[:skin_type].blank? || params[:skin_type] == 'Site') %>
+    </li>
+    <li>
+      <%= span_if_current ts('Public Work Skins'), skins_path(:skin_type => 'WorkSkin') %>
+    </li>
+  <% end %>
+  </ul>
+
+
+<!--/subnav-->

app/views/skins/_wizard_form.html.erb

@@ -1,5 +1,5 @@
 <fieldset class="post skin wizard">
-  <legend><%= ts("Wizard") %></legend>
+  <legend><%= ts('Wizard') %></legend>
   <dl>
     <dt><%= f.label :title, ts('Title') %></dt>
     <dd><%= f.text_field :title %></dd>
@@ -10,7 +10,7 @@
     <dt><%= f.label :margin, ts('Margin width (%)') %> <%= link_to_help('skins-wizard-margin') %></dt>
     <dd><%= f.text_field :margin %></dd>
 
-    <dt><%= f.label :font, ts("Font family") %> <%= link_to_help('skins-wizard-font') %></dt>
+    <dt><%= f.label :font, ts('Font family') %> <%= link_to_help('skins-wizard-font') %></dt>
     <dd><%= f.text_field :font %></dd>
 
     <dt>
@@ -22,7 +22,7 @@
     </dd>
 
     <dt>
-      <%= f.label :foreground_color, t('foreground_color', :default => 'Foreground color (name or hex code)') %> 
+      <%= f.label :foreground_color, ts('Foreground color (name or hex code)') %>
       <%= link_to_help('skins-wizard-foreground-color') %>
     </dt>
     <dd>
@@ -52,7 +52,7 @@
     <dd><%= f.text_field :accent_color %></dd>
   </dl>
 
-    <%= hidden_field_tag "wizard", true %>
+    <%= hidden_field_tag 'wizard', true %>
 </fieldset>
 
 <%= submit_fieldset(f) %>

app/views/skins/index.html.erb

@@ -1,57 +1,53 @@
-<% work_skin = params[:skin_type] && params[:skin_type] == "WorkSkin" %>
-
-<h2 class="heading"><%= @title %></h2>
+<h2 class="heading"><%= ts('Skins') %></h2>
+<%= render 'skin_top_navigation' %>
 
 <p class="notes">
-  <% if work_skin %>
-    With work skins you can customize the way your own works are shown to others. 
-  <% else %>
-    With site skins you can customize <abbr title="Archive Of Our Own">AO3</abbr> for your own browser.
-  <% end %>
-  <%= link_to_help "skins-basics" %>
+  <%= ts('A site skin lets you change the way the Archive looks in your browser, so you can customize your reading experience. With work skins you can customize the way your own works are shown to others.') %>
+  <%= link_to_help 'skins-basics' %>
 </p>
 
-<%= render :partial => "skin_top_navigation" %>
 
-<h3 class="landmark heading">List of Skins</h3>
+<h3 class="heading"><%= @title %></h3>
+<%= render 'skin_type_navigation' %>
+
 
+<h4 class="landmark heading"><%= ts("List of Skins") %></h4>
 <% if @skins.empty? %>
   <p>
-    No <%= work_skin ? "work" : "site" %> skins here yet! 
-    <% if logged_in? %>Why not try <%= link_to ts('making one'), new_skin_path(:skin_type => work_skin ? "WorkSkin" : "Skin") %>?<% end %>
+    <%= params[:work_skins] ? ts('No work skins here yet!') : ts('No site skins here yet!') %>
+    <% if logged_in? %><%= link_to ts('Why not try making one?'), new_skin_path(:skin_type => params[:work_skins] ? 'WorkSkin' : 'Skin') %><% end %>
   </p>
 <% else %>
   <ul class="index group">
   <% for skin in @skins %>
     <li class="skins <% if skin.editable? %>own<% end %> picture blurb group">
       <div class="header module">
         <h4 class="heading" title="title">
-          <%= link_to h(skin.title), skin %>
+          <%= link_to skin.title, skin %>
           <span class="byline">
-            by <%= skin.byline %>
+            <%= ts("by %{byline}", :byline => skin.byline) %>
           </span>
         </h4>
-		    <% if @user %>
+        <% if @user %>
           <% if skin.official? %> 
-            <p class="status approved">(Approved)</p>
+            <p class="status approved">(<%= ts('Approved') %>)</p>
           <% elsif skin.rejected? %>
-            <p class="status declined">(Declined: <%= link_to_help "skins-approval" %> <%= skin.admin_note %>)</p> 
+            <p class="status declined">(<%= ts('Declined:')%> <%= link_to_help 'skins-approval' %> <%= skin.admin_note %>)</p>
           <% elsif skin.public? %> 
-            <p class="status unread">(Not yet reviewed) <%= link_to_help "skins-approval" %></p>
+            <p class="status unread">(<%= ts('Not yet reviewed') %>) <%= link_to_help 'skins-approval' %></p>
           <% end %>
         <% end %>
         <div class="icon"><%= skin_preview_display(skin) %></div>
-		    <p class="datetime"><%= set_format_for_date(skin.created_at) %></p>
+        <p class="datetime"><%= set_format_for_date(skin.created_at) %></p>
       </div>
 
       <!--summary/descriptions-->
-      <h6 class="landmark heading">Description</h6>
+      <h6 class="landmark heading"><%= ts('Description') %></h6>
       <blockquote class="userstuff summary">
-	<!--BACK END:  please enclose the text "No Description Provided" in <p>-->
-        <%=raw skin.description.blank? ? h(ts("(No Description Provided)")) : strip_images(sanitize_field(skin, :description)) %>
+        <%=raw skin.description.blank? ? ts('(No Description Provided)') : strip_images(sanitize_field(skin, :description)) %>
       </blockquote>
 
-      <%= render :partial => "skin_navigation", :locals => {:skin => skin} %>
+      <%= render :partial => 'skin_navigation', :locals => {:skin => skin} %>
 
     </li>
   <% end %>

app/views/skins/new.html.erb

@@ -1,11 +1,18 @@
 <!--BACK END: if this is only accessed via the user dashboard it needs the user dashboard view-->
-<h2 class="heading"><%= ts('Create New Archive Skin')  %></h2>
+<h2 class="heading"><%= ts('Create New Skin')  %></h2>
+
+<%= render :partial => 'skin_top_navigation' %>
 
 <%= error_messages_for :skin %>
 
-<%= render :partial => "skin_top_navigation" %>
+<p class="notes">
+  <%= ts('This form allows you to create a new site or work skin.') %>
+  <%= link_to_help 'skins-basics' %>
+</p>
+
+<%= render :partial => 'skin_type_navigation' %>
 
-<h3 class="landmark heading"><%= ts("New Archive Skin Form") %></h3>
+<h3 class="landmark heading"><%= ts('New Archive Skin Form') %></h3>
 
 <%= form_for @skin, :html => {:multipart => true} do |f| %>
   <%= render 'form', :f => f %>

app/views/skins/new_wizard.html.erb

@@ -1,15 +1,19 @@
 <!--BACK END: if this is only accessed via the user dashboard it needs the user dashboard view-->
-<h2 class="heading"><%= ts('Create New Archive Skin')  %></h2>
+<h2 class="heading"><%= ts('Create New Site Skin')  %></h2>
 
-<%= error_messages_for :skin %>
+<%= render :partial => 'skin_top_navigation' %>
 
-<p class="note"><%= ts("This wizard only creates site skins. You can also %{link} which can be used to add styling to works that you post.", :link => link_to(ts("create a work skin"), url_for(:skin_type => 'WorkSkin'))).html_safe %></p>
+<%= error_messages_for :skin %>
 
-<%= render :partial => "skin_top_navigation" %>
+<p class="notes">
+  <%= ts('This wizard only creates site skins. You can also %{link} which can be used to add styling to works that you post.', :link => link_to(ts('create a work skin'), url_for(:skin_type => 'WorkSkin'))).html_safe %>
+  <%= link_to_help 'skins-basics' %>
+</p>
 
-<h3 class="landmark heading">New Archive Skin Wizard</h3>
+<%= render :partial => 'skin_type_navigation' %>
 
 
+<h3 class="landmark heading"><%= ts('New Archive Site Skin Wizard') %></h3>
 
 <%= form_for @skin do |f| %>
   <%= render 'wizard_form', :f => f %>