From a11fdec9f6857e3c8cd3f8b6b6a1b3edaaae3754 Mon Sep 17 00:00:00 2001 From: Brad Horrocks Date: Thu, 3 Sep 2015 11:09:37 -0600 Subject: [PATCH] Make LTI configurations accept canvas icon class Support passing a class for an icon for styling goodness. added support for `canvas_icon_class` allowing use od css classes from canvas style guide which can make it look better and maintain our styling standards. Fixes PLAT-1222 Test Plan: prereq's - "LOR External Tools" feature flag enabled - (notice the lack of lti example tool, you dont need it) A new commons icon has been added so you should probably run the following `bundle exec rake brand_configs:clean canvas:compile_assets` Add a new LTI tool to the account using [this xml](https://gist.github.com/defektive/dbd182cb04500e236bde) check placements to make sure they show a fancy new icon instead of a crappy star. course_home_sub_navigation should show up as the commons icon placements to look at :assignment_menu - cog on an assignment :course_home_sub_navigation - right side of page :course_settings_sub_navigation :discussion_topic_menu :file_menu (no icon) :global_navigation (no icon) :module_menu :quiz_menu :wiki_page_menu Change-Id: Ib462e928a5a3811df490f9bf2441b755db2882a9 Reviewed-on: https://gerrit.instructure.com/62492 Tested-by: Jenkins Reviewed-by: Nathan Mills QA-Review: August Thornton Product-Review: Brad Horrocks --- .fontcustom-manifest.json | 8 +- app/controllers/application_controller.rb | 26 +++--- app/controllers/assignments_controller.rb | 2 +- .../content_migrations_controller.rb | 2 +- app/controllers/courses_controller.rb | 2 +- app/controllers/external_tools_controller.rb | 40 ++++----- app/helpers/application_helper.rb | 1 + app/models/context_external_tool.rb | 11 ++- app/stylesheets/components/_canvas-icons.scss | 31 +++++-- app/views/assignments/show.html.erb | 2 +- .../_context_module_next.html.erb | 2 +- .../_module_item_next.html.erb | 2 +- app/views/courses/_settings_sidebar.html.erb | 4 +- app/views/courses/show.html.erb | 4 +- app/views/discussion_topics/show.html.erb | 2 +- app/views/external_tools/_icon.html.erb | 10 +++ .../DiscussionTopics/discussion.handlebars | 9 +- .../_external_tool_menuitem.handlebars | 10 +++ .../_external_tools_menu.handlebars | 8 ++ .../assignments/AssignmentListItem.handlebars | 9 +- app/views/jst/quizzes/QuizItemView.handlebars | 9 +- app/views/jst/wiki/WikiPage.handlebars | 9 +- .../jst/wiki/WikiPageIndexItem.handlebars | 9 +- app/views/quizzes/quizzes/show.html.erb | 2 +- lib/tasks/icons.rake | 4 +- public/fonts/canvas/canvas-icons.eot | Bin 32222 -> 32386 bytes public/fonts/canvas/canvas-icons.svg | 23 +++-- public/fonts/canvas/canvas-icons.ttf | Bin 32032 -> 32196 bytes public/fonts/canvas/canvas-icons.woff | Bin 19036 -> 19116 bytes public/fonts/icons/_canvas-icons.scss | 27 ++++-- public/fonts/icons/commons.svg | 24 +++++ .../instructure_external_tools/plugin.js | 24 ++++- .../PluginSpec.coffee | 8 +- .../application_controller_spec.rb | 82 ++++++++++++++++-- spec/helpers/application_helper_spec.rb | 8 +- spec/spec_helper.rb | 1 + .../courses/settings_sidebar.html.erb_spec.rb | 1 + 37 files changed, 288 insertions(+), 128 deletions(-) create mode 100644 app/views/external_tools/_icon.html.erb create mode 100644 app/views/jst/ExternalTools/_external_tool_menuitem.handlebars create mode 100644 app/views/jst/ExternalTools/_external_tools_menu.handlebars create mode 100644 public/fonts/icons/commons.svg diff --git a/.fontcustom-manifest.json b/.fontcustom-manifest.json index 25a9843ce6c..ee1cfd66df6 100644 --- a/.fontcustom-manifest.json +++ b/.fontcustom-manifest.json @@ -1,7 +1,7 @@ { "checksum": { - "previous": "348f51d5e794468e11a57af4c5fe610cc6c673a5e45538fb058946d99836c6ab", - "current": "348f51d5e794468e11a57af4c5fe610cc6c673a5e45538fb058946d99836c6ab" + "previous": "2e26ccf447e726d45ebc10e81051358dbb016e7193463a6ffd9785df95970a5b", + "current": "2e26ccf447e726d45ebc10e81051358dbb016e7193463a6ffd9785df95970a5b" }, "fonts": [ "public/fonts/canvas/canvas-icons.ttf", @@ -118,6 +118,10 @@ "codepoint": 61722, "source": "public/fonts/icons/collection-save.svg" }, + "commons": { + "codepoint": 61879, + "source": "public/fonts/icons/commons.svg" + }, "complete": { "codepoint": 61723, "source": "public/fonts/icons/complete.svg" diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b96c394fa47..5a9ec2b7223 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -151,22 +151,28 @@ class ApplicationController < ActionController::Base return [] if context.is_a?(Group) context = context.account if context.is_a?(User) - tools = ContextExternalTool.all_tools_for(context, {:type => type, + tools = ContextExternalTool.all_tools_for(context, {:placements => type, :root_account => @domain_root_account, :current_user => @current_user}) - extension_settings = [:icon_url] + custom_settings tools.map do |tool| - hash = { - :title => tool.label_for(type, I18n.locale), - :base_url => named_context_url(context, :context_external_tool_path, tool, :launch_type => type) - } - extension_settings.each do |setting| - hash[setting] = tool.extension_setting(type, setting) - end - hash + external_tool_display_hash(tool, type, context, custom_settings) end end + def external_tool_display_hash(tool, type, context=@context, custom_settings=[]) + hash = { + :title => tool.label_for(type, I18n.locale), + :base_url => polymorphic_url([context, :external_tool], id: tool.id, launch_type: type) + } + + extension_settings = [:icon_url, :canvas_icon_class] | custom_settings + extension_settings.each do |setting| + hash[setting] = tool.extension_setting(type, setting) + end + hash + end + helper_method :external_tool_display_hash + def k12? @domain_root_account && @domain_root_account.feature_enabled?(:k12) end diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb index 0dd758d8892..15750feeb7c 100644 --- a/app/controllers/assignments_controller.rb +++ b/app/controllers/assignments_controller.rb @@ -128,7 +128,7 @@ class AssignmentsController < ApplicationController end if @assignment.submission_types.include?("online_upload") || @assignment.submission_types.include?("online_url") - @external_tools = ContextExternalTool.all_tools_for(@context, :user => @current_user, :type => :homework_submission) + @external_tools = ContextExternalTool.all_tools_for(@context, :user => @current_user, :placements => :homework_submission) else @external_tools = [] end diff --git a/app/controllers/content_migrations_controller.rb b/app/controllers/content_migrations_controller.rb index e93352fa526..c255fb1a83b 100644 --- a/app/controllers/content_migrations_controller.rb +++ b/app/controllers/content_migrations_controller.rb @@ -154,7 +154,7 @@ class ContentMigrationsController < ApplicationController options = @plugins.map{|p| {:label => p.metadata(:select_text), :id => p.id}} - external_tools = ContextExternalTool.all_tools_for(@context, :type => :migration_selection, :root_account => @domain_root_account, :current_user => @current_user) + external_tools = ContextExternalTool.all_tools_for(@context, :placements => :migration_selection, :root_account => @domain_root_account, :current_user => @current_user) options.concat(external_tools.map do |et| { id: et.asset_string, diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index 6a3ab7848fa..c2847977e6e 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -1573,7 +1573,7 @@ class CoursesController < ApplicationController @recent_feedback = (@current_user && @current_user.recent_feedback(:contexts => @contexts)) || [] end - @course_home_sub_navigation_tools = ContextExternalTool.all_tools_for(@context, :type => :course_home_sub_navigation, :root_account => @domain_root_account, :current_user => @current_user) + @course_home_sub_navigation_tools = ContextExternalTool.all_tools_for(@context, :placements => :course_home_sub_navigation, :root_account => @domain_root_account, :current_user => @current_user) unless @context.grants_right?(@current_user, session, :manage_content) @course_home_sub_navigation_tools.reject! { |tool| tool.course_home_sub_navigation(:visibility) == 'admins' } end diff --git a/app/controllers/external_tools_controller.rb b/app/controllers/external_tools_controller.rb index 8701ba6c12b..98c09eca538 100644 --- a/app/controllers/external_tools_controller.rb +++ b/app/controllers/external_tools_controller.rb @@ -643,14 +643,14 @@ class ExternalToolsController < ApplicationController # # This would create a tool on this course with two custom fields and a course navigation tab # curl 'https:///api/v1/courses//external_tools' \ - # -H "Authorization: Bearer " \ - # -F 'name=LTI Example' \ - # -F 'consumer_key=asdfg' \ - # -F 'shared_secret=lkjh' \ + # -H "Authorization: Bearer " \ + # -F 'name=LTI Example' \ + # -F 'consumer_key=asdfg' \ + # -F 'shared_secret=lkjh' \ # -F 'url=https://example.com/ims/lti' \ - # -F 'privacy_level=name_only' \ - # -F 'custom_fields[key1]=value1' \ - # -F 'custom_fields[key2]=value2' \ + # -F 'privacy_level=name_only' \ + # -F 'custom_fields[key1]=value1' \ + # -F 'custom_fields[key2]=value2' \ # -F 'course_navigation[text]=Course Materials' \ # -F 'course_navigation[default]=false' # -F 'course_navigation[enabled]=true' @@ -659,12 +659,12 @@ class ExternalToolsController < ApplicationController # # This would create a tool on the account with navigation for the user profile page # curl 'https:///api/v1/accounts//external_tools' \ - # -H "Authorization: Bearer " \ - # -F 'name=LTI Example' \ - # -F 'consumer_key=asdfg' \ - # -F 'shared_secret=lkjh' \ + # -H "Authorization: Bearer " \ + # -F 'name=LTI Example' \ + # -F 'consumer_key=asdfg' \ + # -F 'shared_secret=lkjh' \ # -F 'url=https://example.com/ims/lti' \ - # -F 'privacy_level=name_only' \ + # -F 'privacy_level=name_only' \ # -F 'user_navigation[url]=https://example.com/ims/lti/user_endpoint' \ # -F 'user_navigation[text]=Something Cool' # -F 'user_navigation[enabled]=true' @@ -673,11 +673,11 @@ class ExternalToolsController < ApplicationController # # This would create a tool on the account with configuration pulled from an external URL # curl 'https:///api/v1/accounts//external_tools' \ - # -H "Authorization: Bearer " \ - # -F 'name=LTI Example' \ - # -F 'consumer_key=asdfg' \ - # -F 'shared_secret=lkjh' \ - # -F 'config_type=by_url' \ + # -H "Authorization: Bearer " \ + # -F 'name=LTI Example' \ + # -F 'consumer_key=asdfg' \ + # -F 'shared_secret=lkjh' \ + # -F 'config_type=by_url' \ # -F 'config_url=https://example.com/ims/lti/tool_config.xml' def create if authorized_action(@context, @current_user, :update) @@ -705,8 +705,8 @@ class ExternalToolsController < ApplicationController # # This would update the specified keys on this external tool # curl -X PUT 'https:///api/v1/courses//external_tools/' \ - # -H "Authorization: Bearer " \ - # -F 'name=Public Example' \ + # -H "Authorization: Bearer " \ + # -F 'name=Public Example' \ # -F 'privacy_level=public' def update @tool = @context.context_external_tools.active.find(params[:id] || params[:external_tool_id]) @@ -776,7 +776,7 @@ class ExternalToolsController < ApplicationController def set_tool_attributes(tool, params) attrs = ContextExternalTool::EXTENSION_TYPES - attrs += [:name, :description, :url, :icon_url, :domain, :privacy_level, :consumer_key, :shared_secret, + attrs += [:name, :description, :url, :icon_url, :canvas_icon_class, :domain, :privacy_level, :consumer_key, :shared_secret, :custom_fields, :custom_fields_string, :text, :config_type, :config_url, :config_xml, :not_selectable, :app_center_id] attrs.each do |prop| tool.send("#{prop}=", params[prop]) if params.has_key?(prop) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 812283847de..6025054c041 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -459,6 +459,7 @@ module ApplicationHelper :id => tool.id, :url => tool.editor_button(:url), :icon_url => tool.editor_button(:icon_url), + :canvas_icon_class => tool.editor_button(:canvas_icon_class), :width => tool.editor_button(:selection_width), :height => tool.editor_button(:selection_height) } diff --git a/app/models/context_external_tool.rb b/app/models/context_external_tool.rb index 2fca465cadf..da3379797d6 100644 --- a/app/models/context_external_tool.rb +++ b/app/models/context_external_tool.rb @@ -80,7 +80,7 @@ class ContextExternalTool < ActiveRecord::Base hash[:enabled] = Canvas::Plugin.value_to_boolean(hash[:enabled]) if hash[:enabled] settings[type] = {}.with_indifferent_access - extension_keys = [:custom_fields, :default, :display_type, :enabled, :icon_url, + extension_keys = [:custom_fields, :default, :display_type, :enabled, :icon_url, :canvas_icon_class, :selection_height, :selection_width, :text, :url, :message_type] if custom_keys = CUSTOM_EXTENSION_KEYS[type] extension_keys += custom_keys @@ -275,6 +275,13 @@ class ContextExternalTool < ActiveRecord::Base def icon_url settings[:icon_url] end + def canvas_icon_class=(i_url) + settings[:canvas_icon_class] = i_url + end + + def canvas_icon_class + settings[:canvas_icon_class] + end def text=(val) settings[:text] = val @@ -349,7 +356,7 @@ class ContextExternalTool < ActiveRecord::Base end end - settings.delete(:editor_button) if !editor_button(:icon_url) + settings.delete(:editor_button) unless editor_button(:icon_url) || editor_button(:canvas_icon_class) sync_placements!(EXTENSION_TYPES.select{|type| !!settings[type]}.map(&:to_s)) true diff --git a/app/stylesheets/components/_canvas-icons.scss b/app/stylesheets/components/_canvas-icons.scss index 465b82a2cf1..ff7ed1639c6 100644 --- a/app/stylesheets/components/_canvas-icons.scss +++ b/app/stylesheets/components/_canvas-icons.scss @@ -1,11 +1,27 @@ /// /// How to update Icons // - // Add your new svg(s) to app/icons/ - // Edit the template file. Important!! Edit file in public/fonts/_canvas-icons.scss - // Compile your fonts. In your project in terminal run: bundle exec rake icons:compile - // Check the styleguide to make sure they're pulling in /styleguide - // Done! + // TL;DR + // + // Add your new svg(s) to "public/fonts/icons/" + // Add your new icon markup to this template public/fonts/icons/_canvas-icons.scss + // Rebuild icon font with `bundle exec rake icons:compile` + // + // Details + // + // Help! I cant see my icon in "/styleguide"! + // make sure you updated the template above + // + // I can see my entry in "/styleguide", but it looks like an unsupported unicode character (an empty rectangle or a square with hex in it) + // Try doing a clean, rebuild, then restart your server + // `bundle exec rake brand_configs:clean canvas:compile_assets` + // + // I can see my entry in "/styleguide", but it is completely empty where the icon should be + // Your stylesheet isn't being loaded properly. + // make sure your svg should 512x512 + // clear your browser cache + // + // More info can be found at /// /* @styleguide Icons @@ -287,6 +303,7 @@ Either use `` with icon desired icon class added or insert `` inside ` icon-export icon-equella icon-equation + icon-commons */ @@ -296,8 +313,7 @@ Either use `` with icon desired icon class added or insert `` inside `
  • - <% if tool_hash[:icon_url] %><% end %> + <%= render(partial: 'external_tools/icon', locals: {tool: tool_hash, settings_key: :assignment_menu}) %> <%= tool_hash[:title] %>
    • diff --git a/app/views/context_modules/_context_module_next.html.erb b/app/views/context_modules/_context_module_next.html.erb index 95d4982a83d..315fa1cd6e6 100644 --- a/app/views/context_modules/_context_module_next.html.erb +++ b/app/views/context_modules/_context_module_next.html.erb @@ -155,7 +155,7 @@ <% tool_path = course_external_tool_path(@context, tool, launch_type: 'module_menu', :modules => [context_module ? context_module.id : "{{ id }}"]) %> - <% if icon_url = tool.extension_setting(:module_menu, :icon_url) %><% end %> + <%= render(partial: 'external_tools/icon', locals: {tool: tool, settings_key: :module_menu}) %> <%= tool.label_for(:module_menu, I18n.locale) %> diff --git a/app/views/context_modules/_module_item_next.html.erb b/app/views/context_modules/_module_item_next.html.erb index 768aa8fa296..22a58561306 100644 --- a/app/views/context_modules/_module_item_next.html.erb +++ b/app/views/context_modules/_module_item_next.html.erb @@ -171,7 +171,7 @@ diff --git a/app/views/courses/_settings_sidebar.html.erb b/app/views/courses/_settings_sidebar.html.erb index ce913b21621..dcb60b50962 100644 --- a/app/views/courses/_settings_sidebar.html.erb +++ b/app/views/courses/_settings_sidebar.html.erb @@ -2,7 +2,9 @@ <% @course_settings_sub_navigation_tools.each do |tool| %> - + + <%= render(partial: 'external_tools/icon', locals: {tool: tool, settings_key: :course_settings_sub_navigation}) %> + <%= tool.label_for(:course_settings_sub_navigation, I18n.locale) %> <% if tool.integration_type == 'lor' %> <%= t('links.new_badge', 'New') %> diff --git a/app/views/courses/show.html.erb b/app/views/courses/show.html.erb index 1e44b0a35b7..6e05857f8fb 100644 --- a/app/views/courses/show.html.erb +++ b/app/views/courses/show.html.erb @@ -51,7 +51,7 @@ <% @course_home_sub_navigation_tools.each do |tool| %> - + <%= render(partial: 'external_tools/icon', locals: {tool: tool, settings_key: :course_home_sub_navigation}) %> <%= tool.label_for(:course_home_sub_navigation, I18n.locale) %> <% end %> @@ -86,7 +86,7 @@ <% @course_home_sub_navigation_tools.each do |tool| %> - + <%= render(partial: 'external_tools/icon', locals: {tool: tool, settings_key: :course_home_sub_navigation}) %> <%= tool.label_for(:course_home_sub_navigation, I18n.locale) %> <% end %> diff --git a/app/views/discussion_topics/show.html.erb b/app/views/discussion_topics/show.html.erb index 58250648ccf..e3308af6d2e 100644 --- a/app/views/discussion_topics/show.html.erb +++ b/app/views/discussion_topics/show.html.erb @@ -144,7 +144,7 @@ <% (@discussion_topic_menu_tools || []).each do |tool_hash| %>
  • - <% if tool_hash[:icon_url] %><% end %> + <%= render(partial: 'external_tools/icon', locals: {tool: tool_hash, settings_key: :discussion_topic_menu}) %> <%= tool_hash[:title] %>
    • diff --git a/app/views/external_tools/_icon.html.erb b/app/views/external_tools/_icon.html.erb new file mode 100644 index 00000000000..3893cf1f329 --- /dev/null +++ b/app/views/external_tools/_icon.html.erb @@ -0,0 +1,10 @@ +<% if tool && settings_key %> + <% tool = external_tool_display_hash(tool, settings_key) if tool.respond_to?(:extension_setting) %> + + <% if tool[:canvas_icon_class] %> + + <% elsif tool[:icon_url] %> + + <% end %> + +<% end %> \ No newline at end of file diff --git a/app/views/jst/DiscussionTopics/discussion.handlebars b/app/views/jst/DiscussionTopics/discussion.handlebars index 68c410d60b8..94f52d99331 100644 --- a/app/views/jst/DiscussionTopics/discussion.handlebars +++ b/app/views/jst/DiscussionTopics/discussion.handlebars @@ -133,14 +133,7 @@ {{/if}} {{/if}} {{#if ENV.permissions.manage_content}} - {{#each discussion_topic_menu_tools}} -
  • - - {{#if icon_url}}{{/if}} - {{title}} - -
    • - {{/each}} + {{>ExternalTools/external_tools_menu discussion_topic_menu_tools}} {{/if}} {{/if}} diff --git a/app/views/jst/ExternalTools/_external_tool_menuitem.handlebars b/app/views/jst/ExternalTools/_external_tool_menuitem.handlebars new file mode 100644 index 00000000000..4cffba20ada --- /dev/null +++ b/app/views/jst/ExternalTools/_external_tool_menuitem.handlebars @@ -0,0 +1,10 @@ +{{#if this}} + + {{#if canvas_icon_class}} + + {{else}} + {{#if icon_url}}{{/if}} + {{/if}} + {{title}} + +{{/if}} diff --git a/app/views/jst/ExternalTools/_external_tools_menu.handlebars b/app/views/jst/ExternalTools/_external_tools_menu.handlebars new file mode 100644 index 00000000000..3091c26a534 --- /dev/null +++ b/app/views/jst/ExternalTools/_external_tools_menu.handlebars @@ -0,0 +1,8 @@ +{{#if this}} + {{#each this}} +
  • + {{>ExternalTools/external_tool_menuitem this}} +
    • + {{/each}} + +{{/if}} diff --git a/app/views/jst/assignments/AssignmentListItem.handlebars b/app/views/jst/assignments/AssignmentListItem.handlebars index 9844f2ddd91..ea2c655232a 100644 --- a/app/views/jst/assignments/AssignmentListItem.handlebars +++ b/app/views/jst/assignments/AssignmentListItem.handlebars @@ -106,14 +106,7 @@ >{{#t "move"}}Move To…{{/t}} {{/if}} - {{#each menu_tools}} -
  • - - {{#if icon_url}}{{/if}} - {{title}} - -
    • - {{/each}} + {{>ExternalTools/external_tools_menu menu_tools}} diff --git a/app/views/jst/quizzes/QuizItemView.handlebars b/app/views/jst/quizzes/QuizItemView.handlebars index 77d1fefd567..3bc4ec1657b 100644 --- a/app/views/jst/quizzes/QuizItemView.handlebars +++ b/app/views/jst/quizzes/QuizItemView.handlebars @@ -39,14 +39,7 @@
  • {{#t "links.delete"}}Delete{{/t}}
    • - {{#each quiz_menu_tools}} -
  • - - {{#if icon_url}}{{/if}} - {{title}} - -
    • - {{/each}} + {{>ExternalTools/external_tools_menu quiz_menu_tools}} diff --git a/app/views/jst/wiki/WikiPage.handlebars b/app/views/jst/wiki/WikiPage.handlebars index 7b30752af83..bd03c78b32e 100644 --- a/app/views/jst/wiki/WikiPage.handlebars +++ b/app/views/jst/wiki/WikiPage.handlebars @@ -48,14 +48,7 @@ {{#if CAN.READ_REVISIONS}}
  • {{#t "view_page_history_wiki"}}View Page History{{/t}}
    • {{/if}} - {{#each wiki_page_menu_tools}} -
  • - - {{#if icon_url}}{{/if}} - {{title}} - -
    • - {{/each}} + {{>ExternalTools/external_tools_menu wiki_page_menu_tools}} {{/if}} diff --git a/app/views/jst/wiki/WikiPageIndexItem.handlebars b/app/views/jst/wiki/WikiPageIndexItem.handlebars index bcd2de5d49f..dc565158578 100644 --- a/app/views/jst/wiki/WikiPageIndexItem.handlebars +++ b/app/views/jst/wiki/WikiPageIndexItem.handlebars @@ -20,14 +20,7 @@ {{#unless front_page}}
  • {{#t 'menu.use_front_page'}}Use as Front Page{{/t}}
    • {{/unless}} - {{#each wiki_page_menu_tools}} -
  • - - {{#if icon_url}}{{/if}} - {{title}} - -
    • - {{/each}} + {{>ExternalTools/external_tools_menu wiki_page_menu_tools}} diff --git a/app/views/quizzes/quizzes/show.html.erb b/app/views/quizzes/quizzes/show.html.erb index 6289c1ea134..048c631bdf6 100644 --- a/app/views/quizzes/quizzes/show.html.erb +++ b/app/views/quizzes/quizzes/show.html.erb @@ -221,7 +221,7 @@ <% (@quiz_menu_tools || []).each do |tool_hash| %>
  • - <% if tool_hash[:icon_url] %><% end %> + <%= render(partial: 'external_tools/icon', locals: {tool: tool_hash, settings_key: :quiz_menu}) %> <%= tool_hash[:title] %>
    • diff --git a/lib/tasks/icons.rake b/lib/tasks/icons.rake index c2426e7f22a..416497f89ad 100644 --- a/lib/tasks/icons.rake +++ b/lib/tasks/icons.rake @@ -1,11 +1,13 @@ # lib/icons.rake +require 'lib/brandable_css' + namespace :icons do task :compile do puts "Compiling icons..." puts %x(bundle exec fontcustom compile --force) puts "Compiling stylesheets..." - puts %x(bundle exec npm run compile-sass) + BrandableCSS.compile_all! puts "Compiling styleguide..." puts %x(bundle exec rake css:styleguide) end diff --git a/public/fonts/canvas/canvas-icons.eot b/public/fonts/canvas/canvas-icons.eot index aa943d8217e5663b0cf89161f2c9b722a698a338..79b61484c302473445ed8d0764b6889ca0c2c742 100644 GIT binary patch delta 705 zcmZ9KOK1~O6o&sZnaNl!X{EKcgO3JwK+%8|~TlWYp~2`<0KKyGnD+-*h2; zW%QoB^F1JU0_m3*Q)*HgI03^=`%-ghEuo(A4J-f>>qb{I>SUH$$;J3En{A&^GpRSu zo3DY~H$XU*)pGgIGec~(Vvu!=f>;%^NO@%CT+`7n*dnk1@S;QT{Z``<9it`V2gT@N zJu18tm;|L;g6O>j)K_I+x-7CCk$c7(R=b#(Q5H`Mx{FUcOB^lblw zp+HY8Hk{24$K%f6Urd*)wH8Hbsa3gthzCO2Z7mcqwah@Djw)SBbo!T=dk1WIuyH_> z_aMlb(1`1t1xToJwqQR#aTd`?y__X9Q-ZUV#b;pwi4Cl$pP9*l29|EeC{GC3j2oOS zh~N%q5$&XKmS86zXDf^6jc54-9e(Igk%S$C&=~7%Okf;2qq)##^XqETKB(!smKe{W zQd5j@AxZtle4$(JKjiLld3exfY%N9!##qroOZAyzkpx<-3zG{{w_KPQJL&TGnO6c@ UUx)eL<|n}Or;pjU`SOjO zKB#S1d?hFN&S!7#jRDIVfcq2Shk=Rn$Pa*~xt@)tjLgwz=ePgC(k0jR^U1~7ujB7t zz$1WUFm4)i+OyX|H$d&?S~49q+I|ji0s1F)D5i{JhVD@x^AK+f%Sf4z8qOlX!YrUX z&!iXZzh6iH0J%+eoCL9oqvWe>&on(g!(D|N!2bHi;lJ$e(g^LimlUCI-%IF+oyOM{YiG*e7+9rqomAngu&hB$7S z(1L_y1r2x)wWlUhF^P|IprM7@B&vQx*bY*fp|%C+W;$ zHZdQ!MMtz#1bX}WyTrI@S&5jL6=B=VT5vX6-BP~Mt>mU?z#Yr?cm}$wLOF=LYC+IW f*g}6a{_n~+YgK=P diff --git a/public/fonts/canvas/canvas-icons.svg b/public/fonts/canvas/canvas-icons.svg index d5482333d3c..9d931f6fddc 100644 --- a/public/fonts/canvas/canvas-icons.svg +++ b/public/fonts/canvas/canvas-icons.svg @@ -1,13 +1,13 @@ -Created by FontForge 20150504 at Thu Jun 11 15:36:49 2015 - By Jennifer Stern -Copyright (c) 2015, Jennifer Stern +Created by FontForge 20150715 at Fri Sep 11 16:12:17 2015 + By Brad Horrocks +Copyright (c) 2015, Brad Horrocks @@ -22,7 +22,7 @@ Copyright (c) 2015, Jennifer Stern bbox="-32 -64.416 513.008 449.376" underline-thickness="25.6" underline-position="-51.2" - unicode-range="U+0020-F1B6" + unicode-range="U+0020-F1B7" /> - + + /// /* @styleguide Icons @@ -287,6 +303,7 @@ Either use `` with icon desired icon class added or insert `` inside ` icon-export icon-equella icon-equation + icon-commons */ diff --git a/public/fonts/icons/commons.svg b/public/fonts/icons/commons.svg new file mode 100644 index 00000000000..602cbca1ba2 --- /dev/null +++ b/public/fonts/icons/commons.svg @@ -0,0 +1,24 @@ + + + + Slice 1 + Created with Sketch. + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/public/javascripts/tinymce_plugins/instructure_external_tools/plugin.js b/public/javascripts/tinymce_plugins/instructure_external_tools/plugin.js index 3753c90b321..0da18cc9cc7 100644 --- a/public/javascripts/tinymce_plugins/instructure_external_tools/plugin.js +++ b/public/javascripts/tinymce_plugins/instructure_external_tools/plugin.js @@ -64,12 +64,20 @@ define([ * complete with title, cmd, image, and classes */ buttonConfig: function(button){ - return { + var config = { title: button.name, cmd: 'instructureExternalButton' + button.id, - image: button.icon_url, classes: 'widget btn instructure_external_tool_button' }; + + if (button.canvas_icon_class) { + config.icon = 'hack-to-avoid-mce-prefix ' + button.canvas_icon_class; + } else { + // default to image + config.image = button.icon_url; + } + + return config; }, /** @@ -93,8 +101,16 @@ define([ */ clumpedButtonMapping: function(clumpedButtons, onClickHandler){ return clumpedButtons.reduce(function(items, button){ - var key = " " + htmlEscape(button.name); + var key; + + // added data-tool-id='"+ button.id +"' to make elements unique when the have the same name + if (button.canvas_icon_class) { + key = ""; + } else { + // icon_url is implied + key = ""; + } + key += " " + htmlEscape(button.name); items[key] = function() { onClickHandler(button); }; return items; }, {}); diff --git a/spec/coffeescripts/tinymce_plugins/instructure_external_tools/PluginSpec.coffee b/spec/coffeescripts/tinymce_plugins/instructure_external_tools/PluginSpec.coffee index 91eebf5e230..8e39ae7b7c0 100644 --- a/spec/coffeescripts/tinymce_plugins/instructure_external_tools/PluginSpec.coffee +++ b/spec/coffeescripts/tinymce_plugins/instructure_external_tools/PluginSpec.coffee @@ -30,17 +30,17 @@ define [ equal(Object.keys(items).length, 1) test "clumpedButtonMapping uses a callback for onclick handlers", -> - clump = [{icon_url: 'some/url', name: "somebutton"}] + clump = [{icon_url: 'some/url', name: "somebutton", id: 42}] calls = 0 callback = (()-> calls = calls + 1) items = ExternalTools.clumpedButtonMapping(clump, callback) - items[" somebutton"]() + items[" somebutton"]() equal(calls, 1) test "clumpedButtonMapping escapes extremely unlikely XSS attacks", -> - clump = [{icon_url: 'some/url', name: "Name"}] + clump = [{icon_url: 'some/url', name: "Name", id: 42}] items = ExternalTools.clumpedButtonMapping(clump, (->)) - escapedKey = " <script>alert('attacked')</script>Name" + escapedKey = " <script>alert('attacked')</script>Name" equal(Object.keys(items)[0], escapedKey) test "attachClumpedDropdown sets up dropdown closure when clicked out of", -> diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 9e7d76a9e8f..fae3fa890ac 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -586,23 +586,87 @@ describe ApplicationController do tool.account_navigation = {:url => "http://example.com", :icon_url => "http://example.com", :enabled => true} tool.save! - controller.stubs(:named_context_url).returns("http://example.com") + controller.stubs(:polymorphic_url).returns("http://example.com") external_tools = controller.external_tools_display_hashes(:account_navigation, @group) expect(external_tools).to eq([]) end + + it 'returns array of tools if context is not group' do + @course = course_model + tool = @course.context_external_tools.new(:name => "bob", :consumer_key => "test", :shared_secret => "secret", :url => "http://example.com") + tool.account_navigation = {:url => "http://example.com", :icon_url => "http://example.com", :enabled => true, :canvas_icon_class => 'icon-commons'} + tool.save! + + controller.stubs(:polymorphic_url).returns("http://example.com") + external_tools = controller.external_tools_display_hashes(:account_navigation, @course) + + expect(external_tools).to eq([{:title=>"bob", :base_url=>"http://example.com", :icon_url=>"http://example.com", :canvas_icon_class => 'icon-commons'}]) + end end - it 'returns array of tools if context is not group' do - @course = course_model - tool = @course.context_external_tools.new(:name => "bob", :consumer_key => "test", :shared_secret => "secret", :url => "http://example.com") - tool.account_navigation = {:url => "http://example.com", :icon_url => "http://example.com", :enabled => true} - tool.save! + describe 'external_tool_display_hash' do + def tool_settings(setting, include_class=false) + settings_hash = { + url: "http://example.com/?#{setting.to_s}", + icon_url: "http://example.com/icon.png?#{setting.to_s}", + enabled: true + } - controller.stubs(:named_context_url).returns("http://example.com") - external_tools = controller.external_tools_display_hashes(:account_navigation, @course) + settings_hash[:canvas_icon_class] = "icon-#{setting.to_s}" if include_class + settings_hash + end - expect(external_tools).to eq([{:title=>"bob", :base_url=>"http://example.com", :icon_url=>"http://example.com"}]) + before :once do + @course = course_model + @group = @course.groups.create!(:name => "some group") + @tool = @course.context_external_tools.new(:name => "bob", :consumer_key => "test", :shared_secret => "secret", :url => "http://example.com") + + @tool_settings = [ + :user_navigation, :course_navigation, :account_navigation, :resource_selection, + :editor_button, :homework_submission, :migration_selection, :course_home_sub_navigation, + :course_settings_sub_navigation, :global_navigation, + :assignment_menu, :file_menu, :discussion_topic_menu, :module_menu, :quiz_menu, :wiki_page_menu, + :tool_configuration, :link_selection, :assignment_selection, :post_grades + ] + + @tool_settings.each do |setting| + @tool.send("#{setting}=", tool_settings(setting)) + end + @tool.save! + end + + before :each do + controller.stubs(:request).returns(ActionDispatch::TestRequest.new) + controller.instance_variable_set(:@context, @course) + end + + it 'returns a hash' do + hash = controller.external_tool_display_hash(@tool, :account_navigation) + left_over_keys = hash.keys - [:base_url, :title, :icon_url, :canvas_icon_class] + expect(left_over_keys).to eq [] + end + + it 'all settings are correct' do + @tool_settings.each do |setting| + hash = controller.external_tool_display_hash(@tool, setting) + expect(hash[:base_url]).to eq "http://test.host/courses/#{@course.id}/external_tools/#{@tool.id}?launch_type=#{setting.to_s}" + expect(hash[:icon_url]).to eq "http://example.com/icon.png?#{setting.to_s}" + expect(hash[:canvas_icon_class]).to be nil + end + end + + it 'all settings return canvas_icon_class if set' do + @tool_settings.each do |setting| + @tool.send("#{setting}=", tool_settings(setting, true)) + @tool.save! + + hash = controller.external_tool_display_hash(@tool, setting) + expect(hash[:base_url]).to eq "http://test.host/courses/#{@course.id}/external_tools/#{@tool.id}?launch_type=#{setting.to_s}" + expect(hash[:icon_url]).to eq "http://example.com/icon.png?#{setting.to_s}" + expect(hash[:canvas_icon_class]).to eq "icon-#{setting.to_s}" + end + end end end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 3e92cac13e2..79c60bdea91 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -569,22 +569,22 @@ describe ApplicationHelper do @course = course_model @group = @course.groups.create!(:name => "some group") tool = @course.context_external_tools.new(:name => "bob", :consumer_key => "test", :shared_secret => "secret", :url => "http://example.com") - tool.editor_button = {:url => "http://example.com", :icon_url => "http://example.com"} + tool.editor_button = {:url => "http://example.com", :icon_url => "http://example.com", :canvas_icon_class => 'icon-commons'} tool.save! @context = @group - expect(editor_buttons).to eq([{:name=>"bob", :id=>tool.id, :url=>"http://example.com", :icon_url=>"http://example.com", :width=>800, :height=>400}]) + expect(editor_buttons).to eq([{:name=>"bob", :id=>tool.id, :url=>"http://example.com", :icon_url=>"http://example.com", :canvas_icon_class => 'icon-commons', :width=>800, :height=>400}]) end it "should return hash of tools if in course" do @course = course_model tool = @course.context_external_tools.new(:name => "bob", :consumer_key => "test", :shared_secret => "secret", :url => "http://example.com") - tool.editor_button = {:url => "http://example.com", :icon_url => "http://example.com"} + tool.editor_button = {:url => "http://example.com", :icon_url => "http://example.com", :canvas_icon_class => 'icon-commons'} tool.save! controller.stubs(:group_external_tool_path).returns('http://dummy') @context = @course - expect(editor_buttons).to eq([{:name=>"bob", :id=>tool.id, :url=>"http://example.com", :icon_url=>"http://example.com", :width=>800, :height=>400}]) + expect(editor_buttons).to eq([{:name=>"bob", :id=>tool.id, :url=>"http://example.com", :icon_url=>"http://example.com", :canvas_icon_class => 'icon-commons', :width=>800, :height=>400}]) end it "should not include tools from the domain_root_account for users" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 03dc2da2636..a74533a7f91 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -117,6 +117,7 @@ module RSpec::Rails real_controller = controller_class.new real_controller.instance_variable_set(:@_request, @controller.request) + real_controller.instance_variable_set(:@context, @controller.instance_variable_get(:@context)) @controller.real_controller = real_controller # just calling "render 'path/to/view'" by default looks for a partial diff --git a/spec/views/courses/settings_sidebar.html.erb_spec.rb b/spec/views/courses/settings_sidebar.html.erb_spec.rb index de27fe8fefb..c5fcd54797f 100644 --- a/spec/views/courses/settings_sidebar.html.erb_spec.rb +++ b/spec/views/courses/settings_sidebar.html.erb_spec.rb @@ -75,6 +75,7 @@ describe "courses/_settings_sidebar.html.erb" do before do view_context(@course, @user) assigns[:current_user] = @user + @controller.instance_variable_set(:@context, @course) end it "should display all configured tools" do