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

Alt text manager #611

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions app/assets/javascripts/fae/_alt_text_manager.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@

/* global Fae */

/**
* Fae alt text manager
* @namespace altTextManager
*/

Fae.altTextManager = {

ready: function() {
var originalValue = null;
var emptyLable = 'No alt text';
var $altTextLabel = $('.alt_text_label');
var $altTextInputs = $('.alt_text_input');

$('body').on('click', '.alt_text_label', function() {
$altTextInputs.hide();
$altTextLabel.show();
var $label = $(this);
var $input = $label.next('textarea');
var value = $label.text();

if(value !== emptyLable) {
$input.val(value);
originalValue = value;
}

$label.hide();
$input.show().focus();
});

$('body').on('blur', '.alt_text_input', function() {
var $input = $(this);
var $label = $input.prev('span');
var value = $input.val();
var id = $input.data('id');
var url = 'alt_texts/' + id + '/update_alt';

if(value !== originalValue || value !== emptyLable) {
$.post( url, { 'alt': value }, function() {
$label.text(value === '' ? emptyLable : value);
});
}

$input.hide();
$label.show();
});
}

};

1 change: 1 addition & 0 deletions app/assets/javascripts/fae/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
//= require fae/_tables
//= require fae/_deploy
//= require fae/_contrast
//= require fae/_alt_text_manager

//= require fae

Expand Down
3 changes: 2 additions & 1 deletion app/assets/stylesheets/fae/base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
'modules/tables/sticky',
'modules/tables/tooltips',
'modules/tables/pagination',
'modules/tables/image'
'modules/tables/image',
'modules/tables/alt_text_manager'
;

// Modules
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.alt_text_input {
display:none;
}

.alt_text_input {
min-width: 80px;
}
28 changes: 28 additions & 0 deletions app/controllers/fae/alt_texts_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
module Fae
class AltTextsController < ApplicationController

def index
@items = Fae::Image.for_fae_index.page(params[:page])
@parent_model_options = []
Fae::StaticPage.all.each do |page|
@parent_model_options << ["#{page.title} Page", "Fae::StaticPage-#{page.id}"]
end
Fae::Image.pluck(:imageable_type).uniq.each do |model|
next if ['Fae::StaticPage', 'Fae::Option'].include?(model)
@parent_model_options << [model.titleize, model]
end
end

def update_alt
image = Fae::Image.find_by_id(params[:id])
image.update_attribute(:alt, params[:alt])
head :ok
end

def filter
@items = Fae::Image.filter(params).fae_sort(params).page(params[:page])
render :index, layout: false
end

end
end
36 changes: 36 additions & 0 deletions app/models/fae/image.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,41 @@ def recreate_versions
asset.recreate_versions! if Fae.recreate_versions && asset.present?
end

class << self

def for_fae_index
# Workaround for current inability to save images in capybara tests.
# For tests we need to get the image objects regardless of asset presence.
if Rails.env.test?
order(updated_at: :desc)
else
where('asset IS NOT NULL').order(updated_at: :desc)
end
end

def filter(params)
conditions = {}
if params['parent_model'].present?
if params['parent_model'].include?('-')
parent_model, parent_id = params['parent_model'].split('-')
conditions[:imageable_type] = parent_model
conditions[:imageable_id] = parent_id
else
conditions[:imageable_type] = params['parent_model']
end
end
conditions[:attached_as] = params['attached_as'] if params['attached_as'].present?
alt_text_conditions = []
case params['alt_text_presence']
when 'Present'
alt_text_conditions = ['alt IN (?)', ['', nil]]
when 'Missing'
alt_text_conditions = ['alt NOT IN (?)', ['', nil]]
end
for_fae_index.where(conditions).where(alt_text_conditions)
end

end

end
end
45 changes: 45 additions & 0 deletions app/views/fae/alt_texts/index.html.slim
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
header.content-header.js-content-header
h1 = t('fae.application.alt_texts_heading')

main.content
== fae_filter_form search: false, title: t('fae.application.alt_texts_filter'), action: fae.alt_texts_filter_path do
== fae_filter_select :alt_text_presence, options: ['Both', 'Missing', 'Present']
== fae_filter_select :parent_model, options: @parent_model_options
== fae_filter_select :attached_as, options: Fae::Image.pluck(:attached_as).uniq.collect.compact { |a| [a.titleize, a] }
table.js-sort-column
thead
tr
th ID
th data-sorter="false"
th Parent Model
th Parent ID
th Attached As
th.-alt-input Alt Text
th Size
th Modified
tbody
- if @items.present?
- @items.each do |item|
tr
td = item.id
td
- if item.asset.present? && item.asset.url.present?
img srcset=item.asset.url
- if item.imageable_type == 'Fae::StaticPage'
td = "#{item.imageable.title} Page"
td
- else
td = item.imageable_type.titleize
td = item.imageable_id
td = item.attached_as&.titleize
td
span.alt_text_label
= item.alt.present? ? item.alt : 'No alt text'
textarea.alt_text_input data-id=item.id
td = number_to_human_size(item.file_size)
td = fae_date_format item.updated_at

- else
tr: td colspan="8" No items found

/ == fae_paginate @items
1 change: 1 addition & 0 deletions app/views/fae/application/_header.slim
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ header.main-header#js-main-header
- if current_user.super_admin_or_admin?
li: a href=fae.users_path = t('fae.navbar.users')
li: a href=fae.activity_log_path = t('fae.navbar.activity_log')
li: a href=fae.alt_texts_path = t('fae.navbar.alt_text_manager')
- if current_user.super_admin?
li: a href=fae.option_path = t('fae.navbar.root_settings')
- if @option.live_url.present?
Expand Down
3 changes: 3 additions & 0 deletions config/locales/fae.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ en:
your_settings: 'Your Settings'
users: 'Users'
activity_log: 'Activity Log'
alt_text_manager: 'Alt Text Manager'
live_site: 'Live Site'
jump_to: 'Jump to...'
root_settings: 'Root Settings'
Expand Down Expand Up @@ -126,6 +127,8 @@ en:
alt_helper: 'Read by search engines and visually impaired readers.'
application:
activity_log_heading: 'Activity Log'
alt_texts_heading: 'Alt Text Manager'
alt_texts_filter: 'Search Images'
user_absent: 'user no longer exists'
no_logs: 'No logs available.'
view_site: 'View Site'
Expand Down
3 changes: 3 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
end
resources :users
resources :deploy_hooks
post 'alt_texts/:id/update_alt' => 'alt_texts#update_alt'
get 'alt_texts' => 'alt_texts#index', as: 'alt_texts'
post 'alt_texts/filter' => 'alt_texts#filter', as: 'alt_texts_filter'

get 'settings' => 'users#settings', as: 'settings'
get 'deploy' => 'deploy#index', as: 'deploy'
Expand Down
22 changes: 22 additions & 0 deletions spec/features/alt_text_manager_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
require 'spec_helper'

feature 'Alt Text Manager' do

scenario 'alt text can be edited inline', js: true do
admin_login
release = FactoryBot.create(:release)
image = Fae::Image.create({
imageable_type: 'Release',
imageable_id: release.id,
attached_as: 'bottle_shot'
})
visit fae.alt_texts_path
find('span', text: 'No alt text').click
find(:css, ".alt_text_input").set('Actual alt text')
find(:css, ".alt_text_input").trigger('blur')
eventually { expect(page).to have_content('Actual alt text') }
image.reload
expect(image.alt).to eq('Actual alt text')
end

end