Skip to content

Conversation

@ToMESSKa
Copy link
Contributor

@ToMESSKa ToMESSKa commented Dec 3, 2025

…nt types and TypeScript errors in the examples

INSTUI-4866

ISSUE:

  • events in Select are wrongly typed
  • examples in Select therefore show TypeScript errors

TEST PLAN:

  • open the branch locally
  • copy these code examples from README that use event.key or event.keyCode into a new .tsx file in your local repository, they should not show TypeScript errors.
import React, { useState, useRef } from 'react'
import { Select } from '@instructure/ui-select'

/**
 * This file tests all the README examples that use event.key or event.keyCode
 * to ensure they work correctly with TypeScript after our type fixes.
 */

// Example 1: SingleSelectExample (line 47 in README)
export const SingleSelectExample = ({ options }: { options: Array<{ id: string; label: string }> }) => {
  const [inputValue, setInputValue] = useState(options[0].label)
  const [isShowingOptions, setIsShowingOptions] = useState(false)
  const [highlightedOptionId, setHighlightedOptionId] = useState<string | null>(null)
  const [selectedOptionId, setSelectedOptionId] = useState(options[0].id)

  const handleShowOptions = (event: React.KeyboardEvent | React.MouseEvent) => {
    setIsShowingOptions(true)
    if (inputValue || selectedOptionId || options.length === 0) return

    // This is the exact code from the README with type guard
    if ('key' in event) {
      switch (event.key) {
        case 'ArrowDown':
          return console.log('Arrow down')
        case 'ArrowUp':
          return console.log('Arrow up')
      }
    }
  }

  const handleHighlightOption = (
    event: React.KeyboardEvent | React.MouseEvent,
    { id }: { id?: string; direction?: 1 | -1 }
  ) => {
    event.persist()
    setHighlightedOptionId(id || null)
  }

  return (
    <Select
      renderLabel="Single Select"
      inputValue={inputValue}
      isShowingOptions={isShowingOptions}
      onRequestShowOptions={handleShowOptions}
      onRequestHighlightOption={handleHighlightOption}
    >
      {options.map((option) => (
        <Select.Option
          key={option.id}
          id={option.id}
          isHighlighted={option.id === highlightedOptionId}
          isSelected={option.id === selectedOptionId}
        >
          {option.label}
        </Select.Option>
      ))}
    </Select>
  )
}

// Example 2: AutocompleteExample (line 237 in README)
export const AutocompleteExample = ({ options }: { options: Array<{ id: string; label: string }> }) => {
  const [inputValue, setInputValue] = useState('')
  const [isShowingOptions, setIsShowingOptions] = useState(false)
  const [selectedOptionId, setSelectedOptionId] = useState<string | null>(null)

  const handleShowOptions = (event: React.KeyboardEvent | React.MouseEvent) => {
    setIsShowingOptions(true)
    if (inputValue || selectedOptionId || options.length === 0) return

    // Exact code from README with type guard
    if ('key' in event) {
      switch (event.key) {
        case 'ArrowDown':
          return console.log('Arrow down')
        case 'ArrowUp':
          return console.log('Arrow up')
      }
    }
  }

  return (
    <Select
      renderLabel="Autocomplete"
      inputValue={inputValue}
      isShowingOptions={isShowingOptions}
      onRequestShowOptions={handleShowOptions}
    >
      {options.map((option) => (
        <Select.Option key={option.id} id={option.id}>
          {option.label}
        </Select.Option>
      ))}
    </Select>
  )
}

// Example 3: MultipleSelectExample (lines 443 and 504 in README)
export const MultipleSelectExample = ({ options }: { options: Array<{ id: string; label: string }> }) => {
  const [inputValue, setInputValue] = useState('')
  const [isShowingOptions, setIsShowingOptions] = useState(false)
  const [selectedOptionId, setSelectedOptionId] = useState<string[]>(['opt1', 'opt6'])

  const handleShowOptions = (event: React.KeyboardEvent | React.MouseEvent) => {
    setIsShowingOptions(true)
    if (inputValue || options.length === 0) return

    // Exact code from README with type guard
    if ('key' in event) {
      switch (event.key) {
        case 'ArrowDown':
          return console.log('Arrow down')
        case 'ArrowUp':
          return console.log('Arrow up')
      }
    }
  }

  const handleKeyDown = (event: React.KeyboardEvent) => {
    // Exact code from README with type guard (line 504)
    if ('keyCode' in event && event.keyCode === 8) {
      // when backspace key is pressed
      if (inputValue === '' && selectedOptionId.length > 0) {
        // remove last selected option, if input has no entered text
        setSelectedOptionId(selectedOptionId.slice(0, -1))
      }
    }
  }

  return (
    <Select
      renderLabel="Multiple Select"
      inputValue={inputValue}
      isShowingOptions={isShowingOptions}
      onRequestShowOptions={handleShowOptions}
      onKeyDown={handleKeyDown}
    >
      {options.map((option) => (
        <Select.Option key={option.id} id={option.id}>
          {option.label}
        </Select.Option>
      ))}
    </Select>
  )
}

// Example 4: GroupSelectExample (line 669 in README)
export const GroupSelectExample = ({
  options
}: {
  options: Record<string, Array<{ id: string; label: string }>>
}) => {
  const [inputValue, setInputValue] = useState('')
  const [isShowingOptions, setIsShowingOptions] = useState(false)

  const handleShowOptions = (event: React.KeyboardEvent | React.MouseEvent) => {
    setIsShowingOptions(true)
    if (inputValue || Object.keys(options).length === 0) return

    // Exact code from README with type guard
    if ('key' in event) {
      switch (event.key) {
        case 'ArrowDown':
          return console.log('Arrow down - first group option')
        case 'ArrowUp':
          return console.log('Arrow up - last group option')
      }
    }
  }

  return (
    <Select
      renderLabel="Group Select"
      inputValue={inputValue}
      isShowingOptions={isShowingOptions}
      onRequestShowOptions={handleShowOptions}
    >
      {Object.keys(options).map((key, index) => (
        <Select.Group key={index} renderLabel={key}>
          {options[key].map((option) => (
            <Select.Option key={option.id} id={option.id}>
              {option.label}
            </Select.Option>
          ))}
        </Select.Group>
      ))}
    </Select>
  )
}

// Example 5: GroupSelectAutocompleteExample (line 853 in README)
export const GroupSelectAutocompleteExample = ({
  options
}: {
  options: Record<string, Array<{ id: string; label: string }>>
}) => {
  const [inputValue, setInputValue] = useState('')
  const [isShowingOptions, setIsShowingOptions] = useState(false)
  const [selectedOptionId, setSelectedOptionId] = useState<string | null>(null)

  const handleShowOptions = (event: React.KeyboardEvent | React.MouseEvent) => {
    setIsShowingOptions(true)
    if (inputValue || selectedOptionId || Object.keys(options).length === 0) return

    // Exact code from README with type guard
    if ('key' in event) {
      switch (event.key) {
        case 'ArrowDown':
          return console.log('Arrow down')
        case 'ArrowUp':
          return console.log('Arrow up')
      }
    }
  }

  return (
    <Select
      renderLabel="Group Select with autocomplete"
      inputValue={inputValue}
      isShowingOptions={isShowingOptions}
      onRequestShowOptions={handleShowOptions}
    >
      {Object.keys(options).map((key, index) => (
        <Select.Group key={index} renderLabel={key}>
          {options[key].map((option) => (
            <Select.Option key={option.id} id={option.id}>
              {option.label}
            </Select.Option>
          ))}
        </Select.Group>
      ))}
    </Select>
  )
}

// Example 6: Option Icons Example (line 1218 in README)
export const OptionIconsExample = ({ options }: { options: Array<{ id: string; label: string }> }) => {
  const [inputValue, setInputValue] = useState(options[0].label)
  const [isShowingOptions, setIsShowingOptions] = useState(false)
  const [selectedOptionId, setSelectedOptionId] = useState(options[0].id)

  const handleShowOptions = (event: React.KeyboardEvent | React.MouseEvent) => {
    setIsShowingOptions(true)
    if (inputValue || selectedOptionId || options.length === 0) return

    // Exact code from README with type guard
    if ('key' in event) {
      switch (event.key) {
        case 'ArrowDown':
          return console.log('Arrow down')
        case 'ArrowUp':
          return console.log('Arrow up')
      }
    }
  }

  return (
    <Select
      renderLabel="Option Icons"
      inputValue={inputValue}
      isShowingOptions={isShowingOptions}
      onRequestShowOptions={handleShowOptions}
    >
      {options.map((option) => (
        <Select.Option
          key={option.id}
          id={option.id}
          isSelected={option.id === selectedOptionId}
        >
          {option.label}
        </Select.Option>
      ))}
    </Select>
  )
}

// Test with inferred types (most common usage pattern)
export const InferredTypesExample = () => {
  const [isShowingOptions, setIsShowingOptions] = useState(false)

  return (
    <Select
      renderLabel="Inferred Types"
      isShowingOptions={isShowingOptions}
      // Inline handler with inferred types - this is how most users write it
      onRequestShowOptions={(event) => {
        setIsShowingOptions(true)

        // Type guard needed for union type
        if ('key' in event) {
          switch (event.key) {
            case 'ArrowDown':
              console.log('Down')
              break
            case 'ArrowUp':
              console.log('Up')
              break
          }
        }
      }}
      // Test FocusEvent compatibility
      onRequestHideOptions={(event) => {
        setIsShowingOptions(false)
        // Can access common properties
        console.log(event.type)

        // Can check for keyboard-specific properties
        if ('key' in event) {
          console.log('Keyboard event:', event.key)
        }

        // Can check for mouse-specific properties
        if ('button' in event) {
          console.log('Mouse event:', event.button)
        }

        // Can check for focus-specific properties
        if ('relatedTarget' in event) {
          console.log('Focus event:', event.relatedTarget)
        }
      }}
    >
      <Select.Option id="1">Option 1</Select.Option>
      <Select.Option id="2">Option 2</Select.Option>
    </Select>
  )
}

@ToMESSKa ToMESSKa self-assigned this Dec 3, 2025
@github-actions
Copy link

github-actions bot commented Dec 3, 2025

PR Preview Action v1.6.3
Preview removed because the pull request was closed.
2025-12-23 13:00 UTC

return handleHighlightOption(event, {
id: options[options.length - 1].id
})
if ('key' in event) {
Copy link
Contributor Author

@ToMESSKa ToMESSKa Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed a few examples in README a bit to make them TypeScript error-free

@ToMESSKa ToMESSKa force-pushed the INSTUI-4866-selects-handle-show-options-and-perhaps-other-events-are-wrongly-typed branch from b32cfdb to d8ea8da Compare December 3, 2025 14:42
const newOptions = filterOptions(value)
setInputValue(value)
setFilteredOptions(newOptions)
sethHighlightedOptionId(newOptions.length > 0 ? newOptions[0].id : null)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was a typo

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should really type check these somehow...

@ToMESSKa ToMESSKa requested review from balzss and matyasf December 3, 2025 14:53
Copy link
Collaborator

@matyasf matyasf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, just a small addition

Comment on lines 70 to 78
/**
* Callback fired when first option should be highlighted
*/
onRequestHighlightFirstOption?: (event: React.SyntheticEvent) => void
onRequestHighlightFirstOption?: (event: React.KeyboardEvent) => void

/**
* Callback fired when last option should be highlighted
*/
onRequestHighlightLastOption?: (event: React.SyntheticEvent) => void
onRequestHighlightLastOption?: (event: React.KeyboardEvent) => void
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not even knew that we have such event listeners, as I see from the code these are activating when pressing the home and end buttons respectively. Can you add this to the documentation here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated these

const newOptions = filterOptions(value)
setInputValue(value)
setFilteredOptions(newOptions)
sethHighlightedOptionId(newOptions.length > 0 ? newOptions[0].id : null)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should really type check these somehow...

…nt types and TypeScript errors in the examples

INSTUI-4866
@ToMESSKa ToMESSKa force-pushed the INSTUI-4866-selects-handle-show-options-and-perhaps-other-events-are-wrongly-typed branch from d8ea8da to bdd9fe6 Compare December 9, 2025 15:07
@ToMESSKa ToMESSKa requested a review from matyasf December 9, 2025 15:52
Copy link
Contributor

@balzss balzss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good attention to detail, nice job!

@ToMESSKa ToMESSKa merged commit bde40cc into master Dec 23, 2025
10 of 11 checks passed
@ToMESSKa ToMESSKa deleted the INSTUI-4866-selects-handle-show-options-and-perhaps-other-events-are-wrongly-typed branch December 23, 2025 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants