[test-helpers] Add EuiDraggableObject - #9936
Open
steliosmavro wants to merge 7 commits into
Open
Conversation
Adds a WithStableTestSubjects EuiDroppable story fixture (per-item data-test-subj that survives a reorder, unlike the generic examples which default every item to the same subj) to validate against.
Verify testSubj resolves to an enabled drag handle via the data-rfd-drag-handle-draggable-id attribute @hello-pangea/dnd sets, instead of silently no-oping. Adds a spec covering the disabled case.
weronikaolejniczak
added
the
skip-changelog-eui
weronikaolejniczak
self-requested a review
August 25, 2026 10:04
- Drop the dedicated WithStableTestSubjects fixture story; add data-test-subj directly to EuiDroppable's existing Playground story instead, per review — Playground already covers this, no separate story needed. Removes its stale VRT baselines. - Move the [data-rfd-draggable-id] selector into selectors.ts. - Shorten the class/method doc comments; the detail moved to the package README. - Remove the disabled-draggable validation test — it was closer to testing EuiDraggable's own rendering behavior than the helper. Addresses review feedback from @weronikaolejniczak.
weronikaolejniczak
requested changes
Aug 25, 2026
weronikaolejniczak
left a comment
weronikaolejniczak
left a comment
Contributor
There was a problem hiding this comment.
Let's resolve the conflicts with main.
Contributor
Author
|
Thanks for the review @weronikaolejniczak ! I resolved the conflicts from the previously added helper changes. |
weronikaolejniczak
left a comment
weronikaolejniczak
left a comment
Contributor
There was a problem hiding this comment.
- README says "put
testSubjon the handle, not on theEuiDraggablewrapper" but in Playground we put it on<EuiDraggable>. We should update README to be more clear in what cases this is necessary. - PR description is stale with the recent changes.
data-test-subj on <EuiDraggable> in Playground is correct: with the
default customDragHandle={false}, EUI spreads dragHandleProps onto
the same element as draggableProps, so the wrapper IS the handle.
Only a custom drag handle moves it to a separate inner element.
Previous wording implied they're always different elements.
Addresses review feedback from @weronikaolejniczak.
💚 Build Succeeded
History
|
💚 Build Succeeded
History
|
steliosmavro
added a commit
to steliosmavro/eui
that referenced
this pull request
Aug 25, 2026
Per Weronika's review feedback on the EuiDraggableObject PR (elastic#9936): comments should be short pointers to the README, not repeat the full rationale in the code too, and specs should not assert on EuiRange's own behavior (the number input mirroring the slider's value), only on this helper's own disambiguation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
EuiDraggableObject, a Playwright Component Object that keyboard-reordersEuiDraggableitems, following the package's CONTRIBUTING guide.Prototyped and validated against a real Kibana consumer first per the guide's lifecycle. Kibana PR: elastic/kibana#286130 (CI green, 8/8 + 20/20 on a repeat-3 flake check).
API
reorder(steps): focuses the handle, lifts it (Space), moves itstepspositions (ArrowDown/ArrowUp), drops it (Space). Deliberately keyboard-only — that's the accessible interaction@hello-pangea/dnd(whichEuiDraggablewraps) itself supports, and it's how every real consumer already drives it. No mouse-drag simulation.testSubjtargets the item's own drag handle. With the defaultcustomDragHandle={false}that's theEuiDraggableitem itself (EUI spreadsdragHandlePropsonto the same element asdraggablePropsin that case); with a custom handle it's that inner element instead. A component-type guard verifiestestSubjactually resolves to an enabled handle (via thedata-rfd-drag-handle-draggable-idattribute@hello-pangea/dndsets), rather than silently no-oping on a non-handle element.A finding from authoring this
The design initially included a settle-wait after dropping (waiting for a
euiDraggable--isDraggingclass to clear), mirrored from a Kibana consumer that checked for it. That class hasn't existed since this component's styling moved to Emotion — the check was a no-op in both current EUImainand the@elastic/euiversion Kibana has installed. Dropped it rather than replace it with something that verifies nothing; consumers should assert the resulting order with a retryingexpect, which settles on its own.Testing
tsc --noEmitclean. Playwright validation specs pass against Storybook (Display/EuiDroppable→Playground, withdata-test-subjadded to its two items): 3/3, covering forward reorder, backward reorder, and identity persistence after a move.