Improve pathway tree scrolling

less jumping about
also removed the learner_passport_r2 flag

refs VICE-3932
flag=learner_passport

test plan:
  - edit a pathway
  - click around in it
  > expect smoother scrolling to get selected
    milestone to (roughly) the center
  - view a pathway
  > expect only the tree area to scroll and the
    header stays put
  - click on a step
  > expect it to scroll into the middle (more or less),
    to be highlighted, and the details tray to open
  - close the tray
  > expect the step to be un-highlighted

Change-Id: I03033ce99041addda1c95359fc48ce12f3662cdd
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/339547
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Aaron Suggs <aaron.suggs@instructure.com>
QA-Review: Aaron Suggs <aaron.suggs@instructure.com>
Product-Review: Aaron Suggs <aaron.suggs@instructure.com>

app/controllers/learner_passport_controller.rb

@@ -590,7 +590,6 @@ class LearnerPassportController < ApplicationController
 
   def index
     js_env[:FEATURES][:learner_passport] = @domain_root_account.feature_enabled?(:learner_passport)
-    js_env[:FEATURES][:learner_passport_r2] = @domain_root_account.feature_enabled?(:learner_passport_r2)
 
     # hide the breadcrumbs application.html.erb renders
     render html: "<style>.ic-app-nav-toggle-and-crumbs.no-print {display: none;}</style>".html_safe,

app/views/shared/_new_nav_header.html.erb

@@ -184,7 +184,7 @@
         </a>
       </li>
       <% if @current_user.present? &&  @domain_root_account && @domain_root_account.feature_enabled?(:learner_passport) %>
-        <% if @domain_root_account.feature_enabled?(:learner_passport_r2) && (["admin", "teacher"] & @current_user.roles(@domain_root_account)).present? %>
+        <% if (["admin", "teacher"] & @current_user.roles(@domain_root_account)).present? %>
           <li class="menu-item ic-app-header__menu-list-item <%= ' ic-app-header__menu-list-item--active' if active_path?("/users/#{@current_user.id}/passport") %>">
             <a id="global_nav_passport_link" role="button" href="<%="/users/#{@current_user.id}/passport/admin"%>" class="ic-app-header__menu-list-link">
               <div class="menu-item-icon-container" aria-hidden="true">

config/feature_flags/learning_foundations_release_flags.yml

@@ -220,13 +220,6 @@ learner_passport:
   display_name: Learner Passport Prototype
   description: An in-progress working prototype of the Learner Passport feature.
 
-learner_passport_r2:
-  state: hidden
-  applies_to: RootAccount
-  root_opt_in: true
-  display_name: Learner Passport Prototype R2
-  description: An in-progress working prototype of the Learner Passport feature, Release 2.
-
 precise_link_replacements:
   state: hidden
   display_name: Precise link replacement on content migrations

ui/boot/initializers/router.tsx

@@ -60,7 +60,7 @@ const portalRouter = createBrowserRouter(
       {window.ENV.FEATURES.enhanced_rubrics && RubricRoutes}
 
       {window.ENV.FEATURES.learner_passport && LearnerPassportLearnerRoutes}
-      {window.ENV.FEATURES.learner_passport_r2 && LearnerPassportAdminRoutes}
+      {window.ENV.FEATURES.learner_passport && LearnerPassportAdminRoutes}
 
       <Route path="*" element={<></>} />
     </Route>

ui/features/learner_passport/components/admin/Pathways/MilestoneViewDetailsTray.tsx

@@ -23,13 +23,13 @@ import {Heading} from '@instructure/ui-heading'
 import {Text} from '@instructure/ui-text'
 import {Tray} from '@instructure/ui-tray'
 import {View} from '@instructure/ui-view'
-import type {MilestoneData, PathwayBadgeType} from '../../types'
+import type {MilestoneData} from '../../types'
 import {renderCompletionAward} from './edit/AddBadgeTray'
 import MilestoneRequirementCard from './edit/requirements/MilestoneRequirementCard'
 import {DataContext} from './PathwayEditDataContext'
 
 type MilestoneViewTrayProps = {
-  milestone: MilestoneData
+  milestone: MilestoneData | null
   open: boolean
   onClose: () => void
 }
@@ -56,64 +56,66 @@ const MilestoneViewTray = ({milestone, open, onClose}: MilestoneViewTrayProps) =
           </Flex.Item>
         </Flex>
         <Flex.Item shouldGrow={true} shouldShrink={true} overflowY="auto">
-          <View as="div" padding="0 medium medium medium">
-            <View as="div" padding="0 0 medium 0" borderWidth="0 0 small 0">
-              <View as="div" margin="0 0 small 0">
-                <Text as="div" weight="bold">
-                  {milestone.title}
-                </Text>
-              </View>
-              <View as="div" margin="0 0 small 0">
-                <Text as="div">{milestone.description}</Text>
-              </View>
-              <View as="div" margin="0 0 small 0">
-                {!milestone.required && (
+          {milestone && (
+            <View as="div" padding="0 medium medium medium">
+              <View as="div" padding="0 0 medium 0" borderWidth="0 0 small 0">
+                <View as="div" margin="0 0 small 0">
                   <Text as="div" weight="bold">
-                    Optional
+                    {milestone.title}
                   </Text>
-                )}
+                </View>
+                <View as="div" margin="0 0 small 0">
+                  <Text as="div">{milestone.description}</Text>
+                </View>
+                <View as="div" margin="0 0 small 0">
+                  {!milestone.required && (
+                    <Text as="div" weight="bold">
+                      Optional
+                    </Text>
+                  )}
+                </View>
               </View>
-            </View>
-            <View as="div" padding="large 0" borderWidth="0 0 small 0">
-              <Text as="div" weight="bold">
-                Requirements
-              </Text>
-              <View as="div" margin="0 0 small 0">
-                {milestone.requirements.length > 0 ? (
-                  <View as="div" margin="small 0">
-                    {milestone.requirements.map(requirement => (
-                      <View
-                        key={requirement.id}
-                        as="div"
-                        padding="small"
-                        background="secondary"
-                        borderWidth="small"
-                        borderRadius="medium"
-                        margin="0 0 small 0"
-                      >
-                        <MilestoneRequirementCard
-                          key={requirement.id}
-                          variant="view"
-                          requirement={requirement}
-                        />
-                      </View>
-                    ))}
-                  </View>
-                ) : (
-                  <Text as="div">None</Text>
-                )}
-              </View>
-            </View>
-            {milestone.completion_award && (
-              <View as="div" padding="large 0 0 0">
+              <View as="div" padding="large 0" borderWidth="0 0 small 0">
                 <Text as="div" weight="bold">
-                  Completion Award
+                  Requirements
                 </Text>
-                {milestone.completion_award &&
-                  renderCompletionAward(allBadges, milestone.completion_award)}
+                <View as="div" margin="0 0 small 0">
+                  {milestone.requirements.length > 0 ? (
+                    <View as="div" margin="small 0">
+                      {milestone.requirements.map(requirement => (
+                        <View
+                          key={requirement.id}
+                          as="div"
+                          padding="small"
+                          background="secondary"
+                          borderWidth="small"
+                          borderRadius="medium"
+                          margin="0 0 small 0"
+                        >
+                          <MilestoneRequirementCard
+                            key={requirement.id}
+                            variant="view"
+                            requirement={requirement}
+                          />
+                        </View>
+                      ))}
+                    </View>
+                  ) : (
+                    <Text as="div">None</Text>
+                  )}
+                </View>
               </View>
-            )}
-          </View>
+              {milestone.completion_award && (
+                <View as="div" padding="large 0 0 0">
+                  <Text as="div" weight="bold">
+                    Completion Award
+                  </Text>
+                  {milestone.completion_award &&
+                    renderCompletionAward(allBadges, milestone.completion_award)}
+                </View>
+              )}
+            </View>
+          )}
         </Flex.Item>
         <Flex.Item align="end" width="100%">
           <View as="div" padding="small medium" borderWidth="small 0 0 0" textAlign="end">

ui/features/learner_passport/components/admin/Pathways/PathwayTreeView.tsx

@@ -90,7 +90,6 @@ const PathwayTreeView = ({
   const [dagEdges, setDagEdges] = useState<JSX.Element[]>([])
   const [rootNodeRef, setRootNodeRef] = useState(null)
   const [viewBox, setViewBox] = useState([0, 0, 0, 0])
-  const [treeRef, setTreeRef] = useState<Element | null>(null)
 
   const [preRendered, setPreRendered] = useState(false)
   const [graphBoxHeights, setGraphBoxHeights] = useState<BoxHeights>({
@@ -101,8 +100,8 @@ const PathwayTreeView = ({
   const [selectedSubtree, setSelectedSubtree] = useState<string[]>(() => {
     return selectedStep ? findSubtreeMilestones(pathway.milestones, selectedStep, []) : []
   })
+  const [scrollOffset, setScrollOffset] = useState({top: 0, left: 0})
 
-  const viewRef = useRef<HTMLDivElement | null>(null)
   const preRenderNodeRef = useRef<HTMLDivElement | null>(null)
   const svgRef = useRef<SVGSVGElement | null>(null)
 
@@ -128,9 +127,13 @@ const PathwayTreeView = ({
     if (svgRef.current) {
       setPreRendered(false)
     }
-    viewRef.current = null
     preRenderNodeRef.current = null
     svgRef.current = null
+
+    const treeRoot = document.getElementById('pathway-tree-view')
+    if (treeRoot) {
+      setScrollOffset({top: treeRoot.scrollTop, left: treeRoot.scrollLeft})
+    }
   }, [g])
 
   useEffect(() => {
@@ -140,7 +143,7 @@ const PathwayTreeView = ({
   const handleSelectBox = useCallback(
     (id: string) => {
       if (!onSelected) return
-      if (id === '0' || id === selectedStep) {
+      if (id === '0') {
         onSelected(null)
       } else {
         const milestone = pathway.milestones.find(m => m.id === id)
@@ -148,7 +151,7 @@ const PathwayTreeView = ({
         onSelected(milestone)
       }
     },
-    [onSelected, pathway.milestones, selectedStep]
+    [onSelected, pathway.milestones]
   )
 
   const handleBoxClick = useCallback(
@@ -167,13 +170,7 @@ const PathwayTreeView = ({
     },
     [handleSelectBox]
   )
-  const boxStyle = useMemo(() => {
-    return onSelected
-      ? {
-          cursor: 'pointer',
-        }
-      : {}
-  }, [onSelected])
+
   const boxProps = useMemo(() => {
     return onSelected
       ? {
@@ -192,14 +189,6 @@ const PathwayTreeView = ({
     [setRootNodeRef]
   )
 
-  const isInSelectedBranch = useCallback(
-    (nodeId: string) => {
-      if (!selectedStep) return false
-      if (nodeId === selectedStep) return true
-    },
-    [selectedStep]
-  )
-
   const renderPathwayBoxContent = useCallback(
     (node: GraphNode, type: NodeType, selected: boolean, width: number, height?: number) => {
       if (node.id === 'blank') {
@@ -417,7 +406,7 @@ const PathwayTreeView = ({
               display: 'relative',
               left: 0,
               top: 0,
-              ...boxStyle,
+              cursor: onSelected && n !== 'blank' ? 'pointer' : 'default',
             }}
             {...boxProps}
           >
@@ -451,12 +440,12 @@ const PathwayTreeView = ({
     setDagNodes(nodes)
   }, [
     boxProps,
-    boxStyle,
     g,
     graphBoxHeights.height,
     graphBoxHeights.milestones,
     handleRootNodeRef,
     layout,
+    onSelected,
     pathway.description,
     pathway.first_milestones,
     pathway.image_url,
@@ -534,6 +523,15 @@ const PathwayTreeView = ({
     }
   }, [pathway.id, version, preRendered])
 
+  useLayoutEffect(() => {
+    if (preRendered) {
+      const treeRoot = document.getElementById('pathway-tree-view')
+      if (treeRoot) {
+        treeRoot.scrollTo(scrollOffset.left, scrollOffset.top)
+      }
+    }
+  }, [preRendered, scrollOffset.left, scrollOffset.top])
+
   useEffect(() => {
     if (preRendered && graphBoxHeights.height > 0) {
       renderDAG()
@@ -545,10 +543,11 @@ const PathwayTreeView = ({
   }, [dagNodes, renderDAGEdges])
 
   const probablyRenderTreeControls = useCallback(() => {
-    if (treeRef === null) return null
+    const treeRoot = document.getElementById('pathway-tree-view')
+    if (treeRoot === null) return null
     if (!renderTreeControls) return null
 
-    const box = treeRef.getBoundingClientRect()
+    const box = treeRoot.getBoundingClientRect()
 
     return (
       <div
@@ -564,7 +563,7 @@ const PathwayTreeView = ({
         {renderTreeControls()}
       </div>
     )
-  }, [renderTreeControls, treeRef])
+  }, [renderTreeControls])
 
   const graphWidth = g.graph()?.width ? `${g.graph().width * zoomLevel}px` : 'auto'
   const graphHeight = g.graph()?.height ? `${g.graph().height * zoomLevel}px` : 'auto'
@@ -573,9 +572,8 @@ const PathwayTreeView = ({
     <>
       {probablyRenderTreeControls()}
       <View
-        data-compid="pathway-tree-view"
+        id="pathway-tree-view"
         as="div"
-        elementRef={el => setTreeRef(el)}
         height="100%"
         margin="0"
         position="relative"
@@ -593,7 +591,6 @@ const PathwayTreeView = ({
         >
           <View as="div" width="fit-content">
             <div
-              ref={viewRef}
               style={{
                 position: 'relative',
                 padding: '.5rem',

ui/features/learner_passport/components/admin/Pathways/PathwayView.tsx

@@ -34,7 +34,7 @@ type PathwayViewProps = {
 const PathwayView = ({pathway}: PathwayViewProps) => {
   const [zoomLevel, setZoomLevel] = useState(1)
   const [pathwayDetailsOpen, setPathwayDetailsOpen] = useState(false)
-  const [activeMilestone, setActiveMilestone] = useState<MilestoneData>(pathway.milestones[0])
+  const [activeMilestone, setActiveMilestone] = useState<MilestoneData | null>(null)
   const [milestoneDetailsOpen, setMilestoneDetailsOpen] = useState(false)
 
   const handleZoomIn = useCallback(() => {
@@ -45,20 +45,27 @@ const PathwayView = ({pathway}: PathwayViewProps) => {
     setZoomLevel(zoomLevel - 0.1)
   }, [zoomLevel])
 
-  const handleSelectFromTree = useCallback((milestone: MilestoneData | null) => {
-    if (milestone === null) {
-      setMilestoneDetailsOpen(false)
-      setPathwayDetailsOpen(true)
-    } else {
-      // in this context we know it's a MilestoneViewData
-      setActiveMilestone(milestone as MilestoneData)
-      setMilestoneDetailsOpen(true)
-      setPathwayDetailsOpen(false)
-    }
-  }, [])
+  const handleSelectFromTree = useCallback(
+    (milestone: MilestoneData | null) => {
+      if (milestone === null) {
+        setActiveMilestone(null)
+        setMilestoneDetailsOpen(false)
+        setPathwayDetailsOpen(true)
+      } else if (activeMilestone?.id === milestone.id) {
+        setActiveMilestone(null)
+        setMilestoneDetailsOpen(false)
+      } else {
+        setActiveMilestone(milestone as MilestoneData)
+        setMilestoneDetailsOpen(true)
+        setPathwayDetailsOpen(false)
+      }
+    },
+    [activeMilestone]
+  )
 
   const handleCloseMilestoneDetails = useCallback(() => {
     setMilestoneDetailsOpen(false)
+    setActiveMilestone(null)
   }, [])
 
   const renderBuilderControls = useCallback(() => {
@@ -81,7 +88,7 @@ const PathwayView = ({pathway}: PathwayViewProps) => {
         pathway={pathway}
         version="1"
         zoomLevel={zoomLevel}
-        selectedStep={null}
+        selectedStep={activeMilestone?.id || null}
         onSelected={handleSelectFromTree}
         renderTreeControls={renderBuilderControls}
       />

ui/features/learner_passport/components/admin/Pathways/PathwayViewPage.tsx

@@ -18,32 +18,48 @@
 
 import React from 'react'
 import {useLoaderData} from 'react-router-dom'
-import {Breadcrumb} from '@instructure/ui-breadcrumb'
 import {Flex} from '@instructure/ui-flex'
 import {Heading} from '@instructure/ui-heading'
 import {View} from '@instructure/ui-view'
 import type {PathwayDetailData} from '../../types'
+import AdminHeader from '../AdminHeader'
 import PathwayView from './PathwayView'
 
 const PathwayViewPage = () => {
   const pathway = useLoaderData() as PathwayDetailData
 
   return (
-    <Flex as="div" direction="column" gap="small" alignItems="stretch">
-      <View as="div" margin="0 x-large">
-        <Breadcrumb label="You are here:" size="small">
-          <Breadcrumb.Link href={`/users/${ENV.current_user.id}/passport/admin/pathways/dashboard`}>
-            Pathways
-          </Breadcrumb.Link>
-          <Breadcrumb.Link>{pathway.title}</Breadcrumb.Link>
-        </Breadcrumb>
-        <View as="div" margin="0 0 medium 0">
-          <Heading level="h1">{pathway.title}</Heading>
-        </View>
-      </View>
-      <Flex.Item shouldGrow={true}>
-        <View as="div" overflowX="auto" overflowY="visible">
-          <PathwayView pathway={pathway} />
+    <Flex as="div" direction="column" alignItems="stretch" height="100%">
+      <AdminHeader
+        title={<Heading level="h1">{pathway.title}</Heading>}
+        breadcrumbs={[
+          {
+            text: 'Pathways',
+            url: `/users/${ENV.current_user.id}/passport/admin/pathways/dashboard`,
+          },
+          {text: pathway.title},
+        ]}
+      />
+      <Flex.Item shouldGrow={true} shouldShrink={false} overflowY="visible">
+        <View
+          as="div"
+          id="pathway-view"
+          borderWidth="small 0 0 0"
+          height="100%"
+          position="relative"
+        >
+          <div
+            style={{
+              position: 'absolute',
+              top: 0,
+              right: 0,
+              bottom: 0,
+              left: 0,
+              boxSizing: 'border-box',
+            }}
+          >
+            <PathwayView pathway={pathway} />
+          </div>
         </View>
       </Flex.Item>
     </Flex>

ui/features/learner_passport/components/admin/Pathways/edit/PathwayBuilder.tsx

@@ -157,7 +157,9 @@ const PathwayBuilder = ({pathway, mode, onChange}: PathwayBuilderProps) => {
         currentRoot.next_milestones = []
         pathway.timestamp = Date.now()
       }
-      if (step) {
+      if (step?.id === currentRoot?.id) {
+        setCurrentRoot(null)
+      } else if (step) {
         setCurrentRoot(step)
         if (step.next_milestones.length === 0) {
           step.next_milestones.push('blank')
@@ -179,13 +181,7 @@ const PathwayBuilder = ({pathway, mode, onChange}: PathwayBuilderProps) => {
   )
 
   return (
-    <View
-      as="div"
-      data-compid="pathway-builder"
-      borderWidth="small 0 0 0"
-      height="100%"
-      position="relative"
-    >
+    <View as="div" id="pathway-builder" borderWidth="small 0 0 0" height="100%" position="relative">
       <div
         style={{
           position: 'absolute',

ui/features/learner_passport/components/admin/Pathways/edit/PathwayBuilderSidebar.tsx

@@ -124,7 +124,7 @@ const PathwayBuilderSidebar = ({
 
   return (
     <View
-      data-compid="pathway-builder-sidebar"
+      id="pathway-builder-sidebar"
       as="div"
       padding="medium"
       background="secondary"

ui/shared/global/env/EnvCommon.d.ts

@@ -244,11 +244,7 @@ export type BrandAccountFeatureId = 'embedded_release_notes'
  * Feature id exported in ApplicationController that aren't mentioned in
  * JS_ENV_SITE_ADMIN_FEATURES or JS_ENV_ROOT_ACCOUNT_FEATURES or JS_ENV_BRAND_ACCOUNT_FEATURES
  */
-export type OtherFeatureId =
-  | 'canvas_k6_theme'
-  | 'new_math_equation_handling'
-  | 'learner_passport'
-  | 'learner_passport_r2'
+export type OtherFeatureId = 'canvas_k6_theme' | 'new_math_equation_handling' | 'learner_passport'
 
 /**
  * From ApplicationHelper#set_tutorial_js_env