Skip to content

[EuiRange] Migrate to a function component - #9928

Open
franciscodocapa95-stack wants to merge 2 commits into
elastic:mainfrom
franciscodocapa95-stack:refactor/eui-range-function-component
Open

[EuiRange] Migrate to a function component#9928
franciscodocapa95-stack wants to merge 2 commits into
elastic:mainfrom
franciscodocapa95-stack:refactor/eui-range-function-component

Conversation

@franciscodocapa95-stack

Copy link
Copy Markdown

Summary

Migrates EuiRange from a class component to a function component.

  • Replaces class context/theme access with useFormContext() and useEuiTheme().
  • Preserves stable IDs, popover state, track measurement, blur timing, and existing range behavior.
  • Removes the internal EuiRangeClass export and class-instance ref usage.
  • Updates the Storybook interaction to focus the real numeric input.
  • Adds regression coverage for IDs, input/slider synchronization, validity, callbacks, popover timing, track width, and ticks.

Closes #9494.

API Changes

There are no changes to the documented EuiRange API.

The internal EuiRangeClass export and class-instance ref access are removed as part of the class-to-function migration, as discussed in #9494.

Screenshots

No visual changes are expected.

The migrated component was tested locally in Storybook and compared with the current production documentation. Slider/input synchronization, popover behavior, ticks, levels, highlight, tooltip, and keyboard interaction remained consistent.

Impact Assessment

  • 🔴 Breaking changes — What will break? How many usages in Kibana/Cloud UI are impacted?
  • 💅 Visual changes — May impact style overrides; could require visual testing. Explain and estimate impact.
  • 🧪 Test impact — May break functional or snapshot tests (e.g., HTML structure, class names, default values).
  • 🔧 Hard to integrate — If changes require substantial updates to Kibana, please stage the changes and link them here.

Impact level: 🟢 Low

The migration preserves the existing DOM structure, class names, styles, ARIA attributes, documented props, and snapshots.

Release Readiness

  • Documentation: No documentation changes required
  • Figma: No design changes
  • Migration guide: No documented API or consumer migration required
  • Adoption plan: Not applicable; this is an internal component implementation migration

QA instructions for reviewer

  • Confirm slider and numeric input synchronization in both directions.
  • Confirm keyboard interaction.
  • Confirm the input popover opens on focus and retains the existing delayed blur behavior.
  • Confirm ticks, levels, highlight, and tooltip behavior.
  • Confirm invalid/out-of-range values behave as expected.
  • Confirm generated and custom IDs remain stable.
  • Confirm the InputWithPopover Storybook story focuses the real numeric input.

Checklist before marking Ready for Review

  • Filled out all sections above
  • QA: Tested light/dark modes, high contrast, mobile, Chrome/Safari/Edge/Firefox, keyboard-only, screen reader
  • QA: Tested in CodeSandbox and Kibana
  • QA: Tested docs changes
  • Tests: Added/updated Jest, Cypress, and VRT
  • Changelog: No changelog required, as confirmed in [EuiRange] Migrate from class to function component #9494
  • Breaking changes: No breaking change label required

Validation

  • React 17: 37/37 EuiRange tests passed
  • React 18: 37/37 EuiRange tests passed
  • 16/16 snapshots passed on both React versions
  • Full pre-push validation:
    • 367 test suites passed
    • 5,273 tests passed
    • 2,753 snapshots passed
  • TypeScript passed
  • ESLint passed
  • git diff --check passed
  • No snapshots changed

Reviewer checklist

  • Approved Impact Assessment — Acceptable to merge given the consumer impact.
  • Approved Release Readiness — Docs, Figma, and migration info are sufficient to ship.

@franciscodocapa95-stack
franciscodocapa95-stack requested a review from a team as a code owner August 18, 2026 16:22
@cla-checker-service

cla-checker-service Bot commented Aug 18, 2026

Copy link
Copy Markdown

💚 CLA has been signed

@github-actions

Copy link
Copy Markdown

👋 Since this is a community submitted pull request, a Buildkite build has not been started automatically. Would an Elastic organization member please verify the contents of this pull request and kick off a build manually?

@github-actions github-actions Bot added the community contribution (Don't delete - used for automation) label Aug 18, 2026
@weronikaolejniczak

Copy link
Copy Markdown
Contributor

buildkite test this

@elastic-vault-github-plugin-prod

Copy link
Copy Markdown

📷 2 visual difference(s) found

Look at the visual diff below. If everything is expected, run Approve visual changes to update baselines, re-run the job or make appropriate fixes.

See the visual regression testing wiki for more information.

Expand to review

euirange (2 differences)

StoryDiff %BeforeAfterDiff
input with popover desktop 97.26%
input with popover mobile 90.30%

@infra-vault-gh-plugin-prod

infra-vault-gh-plugin-prod Bot commented Aug 24, 2026

Copy link
Copy Markdown

💔 Build Failed

Failed CI Steps

History

@weronikaolejniczak weronikaolejniczak added the skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) label Aug 24, 2026

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.

nit:

Could you update fireEvent to userEvent in the whole file?

trackWidth: 0,
};
const theme = useEuiTheme();
const [id] = useState(() => propsId || htmlIdGenerator()());

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.

nit: We have a dedicated custom hook for this:

Suggested change
const [id] = useState(() => propsId || htmlIdGenerator()());
const generatedId = useGeneratedHtmlId();
const id = propsId || generatedId;

Comment on lines +101 to +102
| React.ChangeEvent<HTMLInputElement>
| React.MouseEvent<HTMLButtonElement>

@weronikaolejniczak weronikaolejniczak Aug 24, 2026

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.

nit:

Let's destructure these types from the import using type keyword instead of using the React namespace. Applies to the rest of the file.

return thePopover ? thePopover : theRange;
}
}
const thePopover = showInputOnly ? (

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.

nit: Can we just name these popover and range? Adding the prefix the looks weird.

<EuiInputPopover
{...inputPopoverProps}
className={classNames('euiRange__popover', inputPopoverProps?.className)}
input={theInput!} // `showInputOnly` confirms existence

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.

nit: Let's not use bangs or assertions. Those are essentially telling the TS that we know better. Let's fix the underlying issue instead.

fullWidth={fullWidth}
isOpen={isPopoverOpen}
closePopover={closePopover}
disableFocusTrap={true}

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.

nit:

This can simply be:

Suggested change
disableFocusTrap={true}
disableFocusTrap

@weronikaolejniczak

Copy link
Copy Markdown
Contributor

blocking:

The VRTs no longer show the range component. We should fix that!

@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

💚 Build Succeeded

@weronikaolejniczak

Copy link
Copy Markdown
Contributor

Hey @franciscodocapa95-stack, are you planning on continuing this community contribution?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community contribution (Don't delete - used for automation) skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[EuiRange] Migrate from class to function component

2 participants