From c3e952a1a4e34e13e824d4cf4c974c8910af37ff Mon Sep 17 00:00:00 2001 From: Dustin Cowles Date: Tue, 14 May 2024 14:58:59 -0700 Subject: [PATCH] add TopNavigationTools closes ADV-104 flag=top_navigation_placement Test plan: - NOTE: You must restart the web container after running Setting.set - PreReqs: - A tool with the `top_navigation` placement - On a Canvas page with the Top Navigation, check the following - Without the tool in the allow list - ENV does not have key `top_navigation_tools` - No tool buttons show up in the navigation bar - With the tool in the allow list by dev key Setting.set("top_navigation_allowed_dev_keys", "global-dev-key-id") - ENV.top_navigation_tools is populated with the tool config - The tool menu button shows up in the top navigation bar - The tool is listed in the menu and can be launched in drawer - Adjust the page to mobile width - Ensure the tool menu shows up in mobile header and can be launched - The tool should open in a tray that overlays the content - Repeat with the tool only in the domain list Setting.set("top_navigation_allowed_dev_keys", "") Setting.set("top_navigation_allowed_launch_domains", "tool-launch-domain") - With the tool in either allow list, add it to the pinned keys list Setting.set("top_navigation_pinned_dev_keys", "global-dev-key-id") - Ensure the tool has a dedicated button in top nav and the tool menu is not rendered. - Repeate with the tool only in the allowd domains list Setting.set("top_navigation_pinned_dev_keys", "") Setting.set("top_navigation_pinned_launch_domains", "tool-launch-domain") - With the tool in the pinned list but not the allowed list, ensure it is not rendered in the top navigation menu and is not in ENV. Change-Id: I267b78316e54dab7f811e3a7d8eaa752668ac5db Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/347352 Tested-by: Service Cloud Jenkins Reviewed-by: Xander Moffatt QA-Review: Xander Moffatt Product-Review: Dustin Cowles --- app/views/layouts/application.html.erb | 6 +- app/views/shared/_new_nav_header.html.erb | 7 + spec/selenium/add_people/add_people_spec.rb | 2 +- .../dashboard/planner/tutorials_spec.rb | 2 +- .../discussions/discussions_edit_page_spec.rb | 2 +- .../discussions/discussions_new_page_spec.rb | 4 +- .../gbhistory_pagination_spec.rb | 3 + .../speedgrader/speedgrader_rubric_spec.rb | 4 +- .../master_courses/blueprint_sidebar_spec.rb | 4 +- .../quizzes_restrictions_teacher_spec.rb | 2 +- .../custom_screen_actions.rb | 7 +- ui/featureBundles.ts | 1 + ui/features/top_navigation_tools/index.tsx | 89 ++++ ui/features/top_navigation_tools/package.json | 6 + .../react/TopNavigationTools.tsx | 128 ++++++ .../__tests__/TopNavigationTools.test.tsx | 116 +++++ .../TopNavigationTools.test.tsx.snap | 408 ++++++++++++++++++ ui/shared/global/env/EnvCommon.d.ts | 17 + .../jquery/event_trackers/question_viewed.js | 3 + .../react/ContentTypeExternalToolDrawer.tsx | 67 +-- .../ContentTypeExternalToolDrawer.test.jsx | 53 +-- 21 files changed, 832 insertions(+), 99 deletions(-) create mode 100644 ui/features/top_navigation_tools/index.tsx create mode 100644 ui/features/top_navigation_tools/package.json create mode 100644 ui/features/top_navigation_tools/react/TopNavigationTools.tsx create mode 100644 ui/features/top_navigation_tools/react/__tests__/TopNavigationTools.test.tsx create mode 100644 ui/features/top_navigation_tools/react/__tests__/__snapshots__/TopNavigationTools.test.tsx.snap diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 42f0950c0b4..feaea747883 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -19,6 +19,7 @@ css_bundle(:instructure_eportfolio) if @eportfolio_view === true css_bundle(:new_user_tutorials) if tutorials_enabled? js_bundle(:navigation_header) unless @headers == false + js_bundle(:top_navigation_tools) if @domain_root_account&.feature_enabled?(:top_navigation_placement) load_blueprint_courses_ui @has_content_notices = load_content_notices @@ -95,7 +96,7 @@ <%# Flash messages must be outside of #application or they won't work in screenreaders with modals open. %> <%= render :partial => 'shared/static_notices' %> <%= render :partial => 'shared/flash_notices' %> -<%if !!@domain_root_account&.feature_enabled?(:external_tool_drawer) %> +<%if @domain_root_account&.feature_enabled?(:top_navigation_placement) %>
<% end %>
@@ -135,6 +136,9 @@ <% end %>
+ <%if @domain_root_account&.feature_enabled?(:top_navigation_placement) %> +
+ <% end %> <% if tutorials_enabled? %>
<% end %> diff --git a/app/views/shared/_new_nav_header.html.erb b/app/views/shared/_new_nav_header.html.erb index e4b369fa2e6..ecf928218ce 100644 --- a/app/views/shared/_new_nav_header.html.erb +++ b/app/views/shared/_new_nav_header.html.erb @@ -22,6 +22,10 @@ <% js_bundle :nav_tourpoints %> <% end %> +<% if !!@domain_root_account&.feature_enabled?(:top_navigation_placement) %> + <% js_bundle :top_navigation_tools %> +<% end %> + <% if k5_user? dashboard_title = t('Home') @@ -63,6 +67,9 @@ <% else %>
<% end %> + <% if !!@domain_root_account&.feature_enabled?(:top_navigation_placement) %> +
+ <% end %> <% if show_blueprint_button? %> + + ) + })} + {menu_tools.length > 0 && ( + + } key="menu"> + {menu_tools.map((tool: Tool) => { + return ( + handleToolClick(val, menu_tools, props.handleToolLaunch)} + key={tool.id} + value={tool.id} + label={I18n.t('Launch %{tool}', {tool: tool.title})} + > + + {getToolIcon(tool)} + {tool.title} + + + ) + })} + + + )} + + ) +} + +export function MobileTopNavigationTools(props: TopNavigationToolsProps) { + return ( + + } + key="menu" + > + {props.tools.map((tool: Tool) => { + return ( + handleToolClick(val, props)} + key={tool.id} + value={tool.id} + label={I18n.t('Launch %{tool}', {tool: tool.title})} + > + + {getToolIcon(tool)} + {tool.title} + + + ) + })} + + ) +} diff --git a/ui/features/top_navigation_tools/react/__tests__/TopNavigationTools.test.tsx b/ui/features/top_navigation_tools/react/__tests__/TopNavigationTools.test.tsx new file mode 100644 index 00000000000..2958db70f30 --- /dev/null +++ b/ui/features/top_navigation_tools/react/__tests__/TopNavigationTools.test.tsx @@ -0,0 +1,116 @@ +/* + * Copyright (C) 2024 - present Instructure, Inc. + * + * This file is part of Canvas. + * + * Canvas is free software: you can redistribute it and/or modify it under + * the terms of the GNU Affero General Public License as published by the Free + * Software Foundation, version 3 of the License. + * + * Canvas is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR + * A PARTICULAR PURPOSE. See the GNU Affero General Public License for more + * details. + * + * You should have received a copy of the GNU Affero General Public License along + * with this program. If not, see . + */ + +import React from 'react' +import {shallow} from 'enzyme' +import {TopNavigationTools, MobileTopNavigationTools} from '../TopNavigationTools' + +describe('TopNavigationTools', () => { + it('renders', () => { + const tools = [ + { + id: '1', + title: 'Tool 1', + icon_url: 'https://instructure.com', + pinned: true, + }, + { + id: '2', + title: 'Tool 2', + pinned: false, + }, + ] + const handleToolLaunch = jest.fn() + const wrapper = shallow( + + ) + expect(wrapper).toMatchSnapshot() + }) + + it('renders with no tools', () => { + const tools = [] + const handleToolLaunch = jest.fn() + const wrapper = shallow( + + ) + expect(wrapper).toMatchSnapshot() + }) + + it('renders with no pinned tools', () => { + const tools = [ + { + id: '1', + title: 'Tool 1', + icon_url: 'https://instructure.com', + pinned: false, + }, + { + id: '2', + title: 'Tool 2', + icon_url: 'https://instructure.com', + pinned: false, + }, + ] + const handleToolLaunch = jest.fn() + const wrapper = shallow( + + ) + expect(wrapper).toMatchSnapshot() + }) +}) + +describe('MobileTopNavigationTools', () => { + it('renders', () => { + const tools = [ + { + id: '1', + title: 'Tool 1', + icon_url: 'https://instructure.com', + pinned: true, + }, + { + id: '2', + title: 'Tool 2', + icon_url: 'https://instructure.com', + pinned: true, + }, + ] + const handleToolLaunch = jest.fn() + const wrapper = shallow( + + ) + expect(wrapper).toMatchSnapshot() + }) +}) + +describe('handleToolClick', () => { + it('finds tool', () => { + const tool = { + id: '1', + title: 'Tool 1', + icon_url: 'https://instructure.com', + pinned: true, + } + const handleToolLaunch = jest.fn() + const wrapper = shallow( + + ) + wrapper.find('Button').simulate('click', {target: {dataset: {toolId: '1'}}}) + expect(handleToolLaunch).toHaveBeenCalledWith(tool) + }) +}) diff --git a/ui/features/top_navigation_tools/react/__tests__/__snapshots__/TopNavigationTools.test.tsx.snap b/ui/features/top_navigation_tools/react/__tests__/__snapshots__/TopNavigationTools.test.tsx.snap new file mode 100644 index 00000000000..6e08409af2f --- /dev/null +++ b/ui/features/top_navigation_tools/react/__tests__/__snapshots__/TopNavigationTools.test.tsx.snap @@ -0,0 +1,408 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`MobileTopNavigationTools renders 1`] = ` + + } + withArrow={true} +> + + + Tool 1 + + Tool 1 + + + + + + Tool 2 + + Tool 2 + + + + +`; + +exports[`TopNavigationTools renders 1`] = ` + + + + + + + } + withArrow={true} + > + + + + + Tool 2 + + + + + + +`; + +exports[`TopNavigationTools renders with no pinned tools 1`] = ` + + + + } + withArrow={true} + > + + + Tool 1 + + Tool 1 + + + + + + Tool 2 + + Tool 2 + + + + + + +`; + +exports[`TopNavigationTools renders with no tools 1`] = ` + +`; diff --git a/ui/shared/global/env/EnvCommon.d.ts b/ui/shared/global/env/EnvCommon.d.ts index f7f6228f691..9a3c98148d8 100644 --- a/ui/shared/global/env/EnvCommon.d.ts +++ b/ui/shared/global/env/EnvCommon.d.ts @@ -41,6 +41,17 @@ type Role = { plural_label: string } +type ToolPlacement = 'top_navigation' + +export type Tool = { + id: string + title: string + base_url: string + icon_url: string + pinned?: boolean + placement?: ToolPlacement +} + export type GroupOutcome = { id: string title: string @@ -210,6 +221,12 @@ export interface EnvCommon { classes?: string }> breadcrumbs: {name: string; url: string}[] + + /** + * Used by ui/features/top_navigation_tools/react/TopNavigationTools.tsx + * and ui/shared/trays/react/ContentTypeExternalToolDrawer.tsx + */ + top_navigation_tools: Tool[] } /** diff --git a/ui/shared/quiz-log-auditing/jquery/event_trackers/question_viewed.js b/ui/shared/quiz-log-auditing/jquery/event_trackers/question_viewed.js index 422238c5eea..74aaf9fae75 100644 --- a/ui/shared/quiz-log-auditing/jquery/event_trackers/question_viewed.js +++ b/ui/shared/quiz-log-auditing/jquery/event_trackers/question_viewed.js @@ -25,6 +25,9 @@ import parseQuestionId from '../util/parse_question_id' export default class QuestionViewed extends EventTracker { install(deliver, scrollContainer = window) { + // The quiz might be inside a drawer layout, in which case we need to + // watch the content inside the drawer layout instead of the window. + scrollContainer = document.getElementById('drawer-layout-content') || scrollContainer let viewed = [] return this.bind( diff --git a/ui/shared/trays/react/ContentTypeExternalToolDrawer.tsx b/ui/shared/trays/react/ContentTypeExternalToolDrawer.tsx index 064321b141d..fa813406855 100644 --- a/ui/shared/trays/react/ContentTypeExternalToolDrawer.tsx +++ b/ui/shared/trays/react/ContentTypeExternalToolDrawer.tsx @@ -27,47 +27,19 @@ import {TruncateText} from '@instructure/ui-truncate-text' import {View} from '@instructure/ui-view' import {handleExternalContentMessages} from '@canvas/external-tools/messages' import ToolLaunchIframe from '@canvas/external-tools/react/components/ToolLaunchIframe' - -type Tool = { - id: string - title: string - base_url: string - icon_url: string -} - -type KnownResourceType = - | 'assignment' - | 'assignment_group' - | 'audio' - | 'discussion_topic' - | 'document' - | 'image' - | 'module' - | 'quiz' - | 'page' - | 'video' - -export type SelectableItem = { - course_id: string - type: KnownResourceType -} +import type {Tool} from '@canvas/global/env/EnvCommon' type Props = { - tool: Tool + tool: Tool | null pageContent: Element pageContentTitle: string pageContentMinWidth: string pageContentHeight: string trayPlacement: string - acceptedResourceTypes: KnownResourceType[] - targetResourceType: KnownResourceType - allowItemSelection: boolean - selectableItems: SelectableItem[] onDismiss: any - onExternalContentReady: any + onResize: any + onExternalContentReady?: any open: boolean - placement: string - extraQueryParams?: {} } export default function ContentTypeExternalToolDrawer({ @@ -77,30 +49,17 @@ export default function ContentTypeExternalToolDrawer({ pageContentMinWidth, pageContentHeight, trayPlacement, - acceptedResourceTypes, - targetResourceType, - allowItemSelection, - selectableItems, onDismiss, + onResize, onExternalContentReady, open, - placement, - extraQueryParams = {}, }: Props) { - const queryParams = { - com_instructure_course_accept_canvas_resource_types: acceptedResourceTypes, - com_instructure_course_canvas_resource_type: targetResourceType, - com_instructure_course_allow_canvas_resource_selection: allowItemSelection, - com_instructure_course_available_canvas_resources: selectableItems, - display: 'borderless', - placement, - ...extraQueryParams, - } + const queryParams = tool ? {display: 'borderless', placement: tool.placement} : {} const prefix = tool?.base_url.indexOf('?') === -1 ? '?' : '&' const iframeUrl = `${tool?.base_url}${prefix}${$.param(queryParams)}` const toolTitle = tool ? tool.title : 'External Tool' - const toolIconUrl = tool ? tool.icon_url : '' - const toolIconAlt = toolTitle ? `${toolTitle} icon` : '' + const toolIconUrl = tool?.icon_url + const toolIconAlt = toolTitle ? `${toolTitle} Icon` : 'Tool Icon' const iframeRef = useRef() const pageContentRef = useRef() @@ -115,6 +74,14 @@ export default function ContentTypeExternalToolDrawer({ [pageContent] ) + useEffect(() => { + window.addEventListener('resize', onResize) + + return () => { + window.removeEventListener('resize', onResize) + } + }, [onResize]) + useEffect( // returns cleanup function: () => handleExternalContentMessages({ready: onExternalContentReady}), @@ -124,7 +91,7 @@ export default function ContentTypeExternalToolDrawer({ return ( - +
{ - const tool = {id: '1', base_url: 'https://one.lti.com/', title: 'First LTI'} + const tool = { + id: '1', + base_url: 'https://lti1.example.com/', + title: 'First LTI', + pinned: true, + placement: 'top_navigation', + } const onDismiss = jest.fn() const onExternalContentReady = jest.fn() const extraQueryParams = {key1: 'value1', key2: 'value2'} @@ -39,17 +45,11 @@ describe('ContentTypeExternalToolDrawer', () => { tool={tool} pageContent={pageContent} pageContentTitle={pageContentTitle} - pageContentMinWidth="" + pageContentMinWidth="40rem" trayPlacement="end" - acceptedResourceTypes={['page', 'module']} - targetResourceType="page" - allowItemSelection={true} - selectableItems={[{id: '1', name: 'module 1'}]} onDismiss={onDismiss} onExternalContentReady={onExternalContentReady} open={true} - placement="wiki_index_menu" - extraQueryParams={extraQueryParams} {...props} /> ) @@ -75,7 +75,7 @@ describe('ContentTypeExternalToolDrawer', () => { let icon_url beforeAll(() => { - icon_url = 'https://one.lti.com/icon.png' + icon_url = 'https://lti1.example.com/icon.png' tool.icon_url = icon_url }) @@ -85,7 +85,7 @@ describe('ContentTypeExternalToolDrawer', () => { it('renders an icon', () => { const {getByAltText} = renderTray() - expect(getByAltText('First LTI icon')).toHaveAttribute('src', icon_url) + expect(getByAltText('First LTI Icon')).toHaveAttribute('src', icon_url) }) }) @@ -103,33 +103,10 @@ describe('ContentTypeExternalToolDrawer', () => { }) }) - describe('constructs iframe src url', () => { - it('adds ? before parameters if none are already present', () => { - expect(tool.base_url).not.toContain('?') - const {getByTestId} = renderTray() - const src = getByTestId('ltiIframe').src - expect(src).toContain(`${tool.base_url}?`) - }) - - it('appends parameters if some exist already', () => { - tool.base_url = 'https://one.lti.com/?launch_type=wiki_index_menu' - const {getByTestId} = renderTray() - const src = getByTestId('ltiIframe').src - expect(src).toContain(`${tool.base_url}&`) - }) - - it('includes expected parameters', () => { - const {getByTestId} = renderTray() - const src = getByTestId('ltiIframe').src - expect(src).toContain('com_instructure_course_accept_canvas_resource_types') - expect(src).toContain('com_instructure_course_canvas_resource_type') - expect(src).toContain('com_instructure_course_allow_canvas_resource_selection') - expect(src).toContain('com_instructure_course_available_canvas_resources') - expect(src).toContain('display') - expect(src).toContain('placement') - // from extraQueryParams - expect(src).toContain('key1=value1') - expect(src).toContain('key2=value2') - }) + it('constructs iframe src url', () => { + expect(tool.base_url).not.toContain('?') + const {getByTestId} = renderTray() + const src = getByTestId('ltiIframe').src + expect(src).toContain(`${tool.base_url}?`) }) })