Skip to content

feat(ui-text-input,ui-select): add contentSpacing 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 contentSpacing 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-09 11:15 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 from 1bbd315 to c1b5831 Compare July 9, 2026 11:03
ToMESSKa and others added 2 commits July 9, 2026 13:11
…ed-tag padding in multiple Select

INSTUI-5079
…lines

InstUI's font stack is `LatoWeb, Lato, "Helvetica Neue", Helvetica, Arial,
sans-serif`, but the regression app loaded no Lato, so text fell back to the
OS font — Helvetica Neue on macOS, DejaVu Sans in the Linux CI container.
Those fonts have different metrics, so layout-driven components (e.g. the
multiple Select with `contentSpacing="even"`, whose height follows tag
wrapping) rendered at different heights locally vs in CI, breaking the diff.

- Self-host Lato via @fontsource/lato (weights 300/400/700) so the stack
  resolves to a real, bundled font identically in every environment.
- Wait for `document.fonts.ready` before each screenshot so captures never
  happen mid font-load in a fallback face.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ToMESSKa ToMESSKa force-pushed the INSTUI-5079-fix-margin-and-padding-on-multiple-select branch from c1b5831 to 172dbb7 Compare July 9, 2026 11:11
github-actions Bot pushed a commit that referenced this pull request Jul 9, 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