Refactor ObsererOptions to use valid children
Use a callback to notify the observer picker when the highlighted option changes, so we no longer need to use a function as a child to access this value. refs LS-3837 flag=none test plan: - Tests pass - Expect the observer picker to behave and look as usual in normal canvas and k5. - Expect not to see errors associated with the observer picker in the console Change-Id: Ic205828bed4882f27ff874b3eef28fd043b19b6d Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/313385 Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> Reviewed-by: Davis Hyer <dhyer@instructure.com> QA-Review: Davis Hyer <dhyer@instructure.com> Product-Review: Davis Hyer <dhyer@instructure.com>
This commit is contained in:
parent
301791da3a
commit
96c6b2c6bf
|
@ -51,7 +51,6 @@ const ignoredErrors = [
|
|||
/An update to %s inside a test was not wrapped in act/,
|
||||
/Can't perform a React state update on an unmounted component/,
|
||||
/contextType was defined as an instance property on %s/,
|
||||
/Functions are not valid as a React child/, // https://instructure.atlassian.net/browse/LS-3837
|
||||
/Function components cannot be given refs/,
|
||||
/Invalid prop `children` supplied to `(Option)`/,
|
||||
/Invalid prop `editorOptions.plugins` of type `string` supplied to `(ForwardRef|RCEWrapper)`/, // https://instructure.atlassian.net/browse/MAT-453
|
||||
|
|
|
@ -17,7 +17,7 @@
|
|||
*/
|
||||
|
||||
import {useScope as useI18nScope} from '@canvas/i18n'
|
||||
import React, {useState, useRef, ReactElement, ReactNode, ChangeEvent} from 'react'
|
||||
import React, {useState, useRef, ReactElement, ReactNode, ChangeEvent, useEffect} from 'react'
|
||||
import {Alert} from '@instructure/ui-alerts'
|
||||
import {Select} from '@instructure/ui-select'
|
||||
import {Spinner} from '@instructure/ui-spinner'
|
||||
|
@ -40,6 +40,7 @@ type Props = {
|
|||
noOptionsValue?: string
|
||||
renderLabel?: string | ReactNode
|
||||
onOptionSelected: (event, optionId: string) => void
|
||||
onHighlightedOptionChange?: (optionId: string | null) => void
|
||||
onInputChange: (event, value) => void
|
||||
onBlur?: (event) => void
|
||||
onFocus?: (event) => void
|
||||
|
@ -61,6 +62,7 @@ export default function CanvasAsyncSelect({
|
|||
noOptionsLabel = '---',
|
||||
noOptionsValue = '',
|
||||
onOptionSelected = () => {},
|
||||
onHighlightedOptionChange = () => {},
|
||||
onInputChange = () => {},
|
||||
onFocus = () => {},
|
||||
onBlur = () => {},
|
||||
|
@ -77,6 +79,11 @@ export default function CanvasAsyncSelect({
|
|||
const [hasFocus, setHasFocus] = useState(false)
|
||||
const [managedInputValue, setManagedInputValue] = useState('')
|
||||
|
||||
useEffect(() => {
|
||||
onHighlightedOptionChange(highlightedOptionId)
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [highlightedOptionId])
|
||||
|
||||
function findOptionById(id: string): ReactElement {
|
||||
let option
|
||||
React.Children.forEach(children, (c: ReactElement) => {
|
||||
|
|
|
@ -35,6 +35,7 @@ import AddStudentModal from './AddStudentModal'
|
|||
import {parseObservedUsersList, parseObservedUsersResponse} from './utils'
|
||||
|
||||
const I18n = useI18nScope('observer_options')
|
||||
const ADD_STUDENT_OPTION_ID = 'new-student-option'
|
||||
|
||||
const ObserverOptions = ({
|
||||
observedUsersList,
|
||||
|
@ -53,6 +54,7 @@ const ObserverOptions = ({
|
|||
const [selectedUser, setSelectedUser] = useState(null)
|
||||
const [newStudentModalOpen, setNewStudentModalOpen] = useState(false)
|
||||
const isOnlyObserver = currentUserRoles?.every(r => r === 'user' || r === 'observer')
|
||||
const [highlightedOption, setHighlightedOption] = useState(null)
|
||||
|
||||
const updateObservedUser = useCallback(
|
||||
user => {
|
||||
|
@ -122,14 +124,16 @@ const ObserverOptions = ({
|
|||
const addStudentOption = (
|
||||
<CanvasAsyncSelect.Option
|
||||
key="new"
|
||||
id="new-student-option"
|
||||
id={ADD_STUDENT_OPTION_ID}
|
||||
value="new"
|
||||
renderBeforeLabel={props => <IconAddLine color={!props.isHighlighted ? 'brand' : null} />}
|
||||
// according to the documentation the next line should override the default color
|
||||
// on inst-ui 8.7.0, but it doesn't seem to work on inst-ui 7.9.0
|
||||
// themeOverride={{color: k5Theme.variables.colors.brand}}
|
||||
>
|
||||
{props => <Text color={!props.isHighlighted ? 'brand' : null}>{I18n.t('Add Student')}</Text>}
|
||||
<Text color={highlightedOption !== ADD_STUDENT_OPTION_ID ? 'brand' : null}>
|
||||
{I18n.t('Add Student')}
|
||||
</Text>
|
||||
</CanvasAsyncSelect.Option>
|
||||
)
|
||||
|
||||
|
@ -168,8 +172,10 @@ const ObserverOptions = ({
|
|||
<View as="div" margin={margin}>
|
||||
<CanvasAsyncSelect
|
||||
autoFocus={!!autoFocus}
|
||||
onHighlightedOptionChange={setHighlightedOption}
|
||||
data-testid="observed-student-dropdown"
|
||||
inputValue={selectSearchValue}
|
||||
isLoading={false}
|
||||
renderLabel={
|
||||
<ScreenReaderContent>
|
||||
{renderLabel || I18n.t('Select a student to view')}
|
||||
|
|
Loading…
Reference in New Issue