Skip to content

feat(ui-text-input,ui-select): add renderBeforeInputElementGap prop for even wrapped-tag padding in multiple Select#2606

Open
ToMESSKa wants to merge 2 commits into
masterfrom
INSTUI-5079-fix-margin-and-padding-on-multiple-select
Open

feat(ui-text-input,ui-select): add renderBeforeInputElementGap prop for even wrapped-tag padding in multiple Select#2606
ToMESSKa wants to merge 2 commits into
masterfrom
INSTUI-5079-fix-margin-and-padding-on-multiple-select

Conversation

@ToMESSKa

@ToMESSKa ToMESSKa commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

INSTUI-5094 (includes regression-test font-determinism fix)

INSTUI-5079

ISSUE:

  • multiple select with multiple rows of Tags, the bottom and top margins aren't consistent
image

-TEST PLAN:

  • check the code below if the examples have consistent top and bottom padding in the legacy and new themes
  • check the disabled state in the code examples too, the tag spacing should work as well
  • Tags in Select/TextInput should work the same when the new prop is not set and the current change should cause a breaking change
const MultiSelectSizeExample = ({ size, options }) => {
    const [inputValue, setInputValue] = useState('')
    const [isShowingOptions, setIsShowingOptions] = useState(false)
    const [highlightedOptionId, setHighlightedOptionId] = useState(null)
    const [selectedOptionId, setSelectedOptionId] = useState(
      // the large version starts with one fewer (5) selected state
      size === 'large'
        ? ['opt1', 'opt2', 'opt3', 'opt4', 'opt5']
        :  ['opt1', 'opt2', 'opt4', 'opt5', 'opt6', 'opt7']
    )
    const inputRef = useRef()

    const getOptionById = (id) => options.find((o) => o.id === id)

    const availableOptions = () =>
      options.filter((o) => !selectedOptionId.includes(o.id))

    const focusInput = () => {
      if (inputRef.current) {
        inputRef.current.blur()
        inputRef.current.focus()
      }
    }

    const handleSelectOption = (event, { id }) => {
      if (!getOptionById(id)) return
      focusInput()
      setSelectedOptionId([...selectedOptionId, id])
      setHighlightedOptionId(null)
      setInputValue('')
      setIsShowingOptions(false)
    }

    const handleKeyDown = (event) => {
      // remove last selected option on backspace, if input has no entered text
      if (event.keyCode === 8 && inputValue === '' && selectedOptionId.length > 0) {
        setHighlightedOptionId(null)
        setSelectedOptionId(selectedOptionId.slice(0, -1))
      }
    }

    const dismissTag = (e, tag) => {
      // prevent closing of list
      e.stopPropagation()
      e.preventDefault()
      setSelectedOptionId(selectedOptionId.filter((id) => id !== tag))
      setHighlightedOptionId(null)
      inputRef.current.focus()
    }

    const renderTags = () =>
      selectedOptionId.map((id) => (
        <Tag
          dismissible
          key={id}
          size={size}
          text={
            <AccessibleContent alt={`Remove ${getOptionById(id).label}`}>
              {getOptionById(id).label}
            </AccessibleContent>
          }
          onClick={(e) => dismissTag(e, id)}
        />
      ))

    return (
      <Select
        renderLabel={`Multiple Select (${size})`}
        size={size}
        width="30rem"
        contentSpacing="even"
        assistiveText="Type or use arrow keys to navigate options. Multiple selections allowed."
        inputValue={inputValue}
        isShowingOptions={isShowingOptions}
        inputRef={(el) => {
          inputRef.current = el
        }}
        onInputChange={(e) => setInputValue(e.target.value)}
        onRequestShowOptions={() => setIsShowingOptions(true)}
        onRequestHideOptions={() => setIsShowingOptions(false)}
        onRequestHighlightOption={(e, { id }) => setHighlightedOptionId(id)}
        onRequestSelectOption={handleSelectOption}
        onKeyDown={handleKeyDown}
        renderBeforeInput={selectedOptionId.length > 0 ? renderTags() : null}
      >
        {availableOptions().length > 0 ? (
          availableOptions().map((option) => (
            <Select.Option
              id={option.id}
              key={option.id}
              isHighlighted={option.id === highlightedOptionId}
            >
              {option.label}
            </Select.Option>
          ))
        ) : (
          <Select.Option id="empty-option" key="empty-option">
            ---
          </Select.Option>
        )}
      </Select>
    )
  }

  const states = [
    { id: 'opt1', label: 'Alabama' },
    { id: 'opt2', label: 'Alaska' },
    { id: 'opt3', label: 'Arizona' },
    { id: 'opt4', label: 'Arkansas' },
    { id: 'opt5', label: 'California' },
    { id: 'opt6', label: 'Colorado' },
    { id: 'opt7', label: 'Connecticut' },
    { id: 'opt8', label: 'Delaware' },
    { id: 'opt9', label: 'Florida' },
    { id: 'opt10', label: 'Georgia' }
  ]

  render(
    <View as="div">
      <MultiSelectSizeExample size="small" options={states} />
      <View as="div" margin="medium 0 0 0">
        <MultiSelectSizeExample size="medium" options={states} />
      </View>
      <View as="div" margin="medium 0 0 0">
        <MultiSelectSizeExample size="large" options={states} />
      </View>
    </View>
  )

@ToMESSKa ToMESSKa self-assigned this Jun 24, 2026
@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://instructure.design/pr-preview/pr-2606/

Built to branch gh-pages at 2026-07-14 09:18 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

Visual regression report

⚠️ Changes detected.

Status Count
Unchanged 3
Changed 29
New 0
Removed 0

📊 View full report

Diff images (29)

alert.png — 3444 pixels differ

badge.png — 1300 pixels differ

billboard.png — 10930 pixels differ

breadcrumb.png — 9136 pixels differ

button-and-derivatives.png — 13604 pixels differ

byline.png — 5878 pixels differ

calendar.png — 7170 pixels differ

checkbox.png — 5095 pixels differ

checkboxgroup.png — 8807 pixels differ

colorpicker.png — 26341 pixels differ

contextview.png — 9344 pixels differ

dateinput-dateinput2.png — 3755 pixels differ

datetimeinput.png — 4776 pixels differ

drilldown.png — 9731 pixels differ

filedrop.png — 20401 pixels differ

form-errors.png — 34246 pixels differ

heading.png — 17393 pixels differ

link.png — 21258 pixels differ

menu.png — 4949 pixels differ

metric-pill-tag-timeselect-text.png — 21956 pixels differ

options.png — 96793 pixels differ

pagination.png — 1692 pixels differ

progressbar.png — 1421 pixels differ

select-simpleselect.png — 7058 pixels differ

table.png — 8866 pixels differ

tabs.png — 2863 pixels differ

tooltip.png — 44534 pixels differ

treebrowser.png — 6805 pixels differ

view.png — 17107 pixels differ

Baselines come from the visual-baselines branch. They refresh on every merge to master.

github-actions Bot pushed a commit that referenced this pull request Jun 24, 2026
@ToMESSKa ToMESSKa changed the title fix(ui-select): fix padding around Tags in multiple Select fix(docs): fix padding around Tags in multiple Select Jun 25, 2026
@ToMESSKa
ToMESSKa force-pushed the INSTUI-5079-fix-margin-and-padding-on-multiple-select branch 2 times, most recently from 15375f4 to 22196a0 Compare July 1, 2026 10:31
github-actions Bot pushed a commit that referenced this pull request Jul 1, 2026
@ToMESSKa
ToMESSKa force-pushed the INSTUI-5079-fix-margin-and-padding-on-multiple-select branch from 22196a0 to 2366123 Compare July 1, 2026 11:11
@ToMESSKa ToMESSKa changed the title fix(docs): fix padding around Tags in multiple Select fix(docs): add contentSpacing prop for even wrapped-tag padding in multiple Select Jul 1, 2026
@ToMESSKa ToMESSKa changed the title fix(docs): add contentSpacing prop for even wrapped-tag padding in multiple Select feat(ui-text-input,ui-select): add contentSpacing prop for even wrapped-tag padding in multiple Select Jul 1, 2026
github-actions Bot pushed a commit that referenced this pull request Jul 1, 2026
@ToMESSKa
ToMESSKa marked this pull request as ready for review July 1, 2026 11:24
<div>
<Select
renderLabel="Multiple Select"
contentSpacing="even"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When contentSpacing="even" is set, the even tag spacing only works while the input is enabled, because beforeElement is display: contents only in the enabled state — in the disabled state it becomes a plain block, so the tags are no longer individual flex items.

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@git-nandor Nice catch, I fixed it, can you please check again?

@ToMESSKa
ToMESSKa force-pushed the INSTUI-5079-fix-margin-and-padding-on-multiple-select branch from 2366123 to 5a58d6b Compare July 1, 2026 15:00
@ToMESSKa
ToMESSKa requested review from git-nandor and removed request for HerrTopi, balzss, joyenjoyer and matyasf July 2, 2026 07:10
@ToMESSKa
ToMESSKa force-pushed the INSTUI-5079-fix-margin-and-padding-on-multiple-select branch 2 times, most recently from 997069c to 738fead Compare July 2, 2026 12:09
github-actions Bot pushed a commit that referenced this pull request Jul 2, 2026
@ToMESSKa
ToMESSKa requested a review from matyasf July 8, 2026 13:29
@ToMESSKa
ToMESSKa force-pushed the INSTUI-5079-fix-margin-and-padding-on-multiple-select branch 2 times, most recently from c1b5831 to 172dbb7 Compare July 9, 2026 11:11
github-actions Bot pushed a commit that referenced this pull request Jul 9, 2026

@matyasf matyasf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also change the example in TextInput to use this prop, change the example not to use margin="xxx-small xxx-small xxx-small none"

Comment thread packages/ui-text-input/src/TextInput/v2/props.ts Outdated
Comment thread packages/ui-select/src/Select/v2/props.ts Outdated
@ToMESSKa
ToMESSKa force-pushed the INSTUI-5079-fix-margin-and-padding-on-multiple-select branch from 172dbb7 to 4d2de15 Compare July 14, 2026 09:13
@ToMESSKa
ToMESSKa requested a review from matyasf July 14, 2026 09:30
@ToMESSKa ToMESSKa changed the title feat(ui-text-input,ui-select): add contentSpacing prop for even wrapped-tag padding in multiple Select feat(ui-text-input,ui-select): add renderBeforeInputElementGap prop for even wrapped-tag padding in multiple Select Jul 15, 2026
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.

3 participants