Skip to content

[Traces][Discover] Prevent flyout remount when switching document types in Trace Waterfall#250406

Merged
lucaslopezf merged 16 commits intoelastic:mainfrom
lucaslopezf:249695-traces-discover-enhance-flyouts-behavior
Jan 30, 2026
Merged

[Traces][Discover] Prevent flyout remount when switching document types in Trace Waterfall#250406
lucaslopezf merged 16 commits intoelastic:mainfrom
lucaslopezf:249695-traces-discover-enhance-flyouts-behavior

Conversation

@lucaslopezf
Copy link
Copy Markdown
Contributor

@lucaslopezf lucaslopezf commented Jan 26, 2026

Closes: #249695

Summary

Previously, the trace waterfall had two separate flyout components for spans and logs. While this design was architecturally valid, it caused a suboptimal user experience: when switching between document types (e.g., clicking an error badge to view a log), the flyout would unmount and remount, causing a visual flash and unnecessary loading states.

Solution: Unified DocumentDetailFlyout

We introduced DocumentDetailFlyout as a single entry point that internally delegates to the appropriate content component based on the document type.

Note: These components are designed specifically for the trace waterfall context and are not intended for reuse elsewhere. If standalone document flyouts are needed in the future, a more generalized abstraction can be considered at that time.

┌─────────────────────────────────────────────────────────────┐
│                    FullScreenWaterfall                       │
│  (manages docId, docIndex, activeFlyoutType, activeSection) │
└─────────────────────────┬───────────────────────────────────┘
                          │
                          ▼
┌─────────────────────────────────────────────────────────────┐
│                   DocumentDetailFlyout                       │
│  - Discriminates by `type` (spanFlyoutId | logsFlyoutId)    │
│  - Calls appropriate data hook                               │
│  - Renders WaterfallFlyout wrapper                          │
└─────────────────────────┬───────────────────────────────────┘
                          │
          ┌───────────────┴───────────────┐
          ▼                               ▼
┌─────────────────────┐       ┌─────────────────────┐
│   SpanFlyoutContent │       │   LogFlyoutContent  │
└─────────────────────┘       └─────────────────────┘

Key Architectural Decisions

1. Hook + Content Separation

Each flyout type is split into:

  • Data hook (useSpanFlyoutData, useLogFlyoutData) - fetches and transforms data
  • Content component (SpanFlyoutContent, LogFlyoutContent) - renders the Overview tab

Why?

  • Allows DocumentDetailFlyout to call both hooks and select the appropriate data
  • Content components receive only what they need (no null checks internally)
  • Easier to test data fetching separately from rendering

Usage from Parent

The parent component (FullScreenWaterfall) only needs to:

  1. Track the document ID and active flyout type
  2. Optionally track the document index (for logs) and active section (for span scroll-to behavior)
  3. Pass the trace dataView for the Table/JSON tabs

The unified flyout handles all the complexity of fetching data and rendering the appropriate content.

Demo

demo.mov

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

  • Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
  • Documentation was added for features that require explanation or tutorials
  • Unit or functional tests were updated or added to match the most common scenarios
  • If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
  • This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
  • Flaky Test Runner was used on any tests changed
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
  • Review the backport guidelines and apply applicable backport:* labels.

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

@lucaslopezf
Copy link
Copy Markdown
Contributor Author

/ci

@lucaslopezf lucaslopezf force-pushed the 249695-traces-discover-enhance-flyouts-behavior branch from f8d8580 to b45524d Compare January 26, 2026 17:07
@lucaslopezf lucaslopezf changed the title 249695 traces discover enhance flyouts behavior [Traces][Discover] Flyout remounts when switching between Span and Log document types Jan 26, 2026
@lucaslopezf
Copy link
Copy Markdown
Contributor Author

/ci

@lucaslopezf lucaslopezf changed the title [Traces][Discover] Flyout remounts when switching between Span and Log document types [Traces][Discover] Prevent flyout remount when switching document types in Trace Waterfall Jan 26, 2026
}

export function useSpanFlyoutData({ spanId, traceId }: UseSpanFlyoutDataParams): SpanFlyoutData {
const { span, loading } = useFetchSpan({ spanId, traceId });
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.

Note for reviewer:

Code reference:

const { span, loading } = useFetchSpan({ spanId, traceId });
const { indexes } = useDataSourcesContext();
const [flyoutRef, setFlyoutRef] = useState<OverviewApi | null>(null);
const documentAsHit = useMemo<DataTableRecord | null>(() => {
if (!span) return null;
return {
id: span._id,
raw: {
_index: span._index,
_id: span._id,
_source: span as unknown as Record<string, unknown>,
},
flattened: flattenObject(span),
};
}, [span]);
const isSpan = isSpanHit(documentAsHit);


const title = i18n.translate(
'unifiedDocViewer.observability.traces.fullScreenWaterfall.spanFlyout.title',
{
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.

<WaterfallFlyout
flyoutId={spanFlyoutId}
onCloseFlyout={onCloseFlyout}
<Overview
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.

Note for reviewer:

Code reference:

<Overview
ref={setFlyoutRef}
hit={documentAsHit}
indexes={indexes}
showWaterfall={false}
showActions={false}
dataView={dataView}

}

export function useLogFlyoutData({ id, index }: UseLogFlyoutDataParams): LogFlyoutData {
const { loading, log, index: resolvedIndex } = useFetchLog({ id, index });
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.

note for reviewer:

Code reference:

const { loading, log, index: resolvedIndex } = useFetchLog({ id, index });
const { indexes } = useDataSourcesContext();
const {
dataView: logDataView,
error,
loading: loadingDataView,
} = useAdhocDataView({ index: resolvedIndex ?? null });
const documentAsHit = useMemo<DataTableRecord | null>(() => {
if (!log || !id || !resolvedIndex) return null;
return {
id,
raw: {
_index: resolvedIndex,
_id: id,
_source: log,
},
flattened: flattenObject(log),
};
}, [id, log, resolvedIndex]);

}, [id, log, resolvedIndex]);

const title = i18n.translate(
'unifiedDocViewer.observability.traces.fullScreenWaterfall.logFlyout.title.log',
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.

/>
) : null}
</WaterfallFlyout>
<LogsOverview hit={hit} dataView={logDataView} indexes={indexes} showTraceWaterfall={false} />
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.

@lucaslopezf
Copy link
Copy Markdown
Contributor Author

/ci

@lucaslopezf
Copy link
Copy Markdown
Contributor Author

/oblt-deploy

@lucaslopezf lucaslopezf added release_note:enhancement Team:obs-exploration Observability Exploration team v9.4.0 backport:skip This PR does not require backporting labels Jan 27, 2026
@lucaslopezf lucaslopezf marked this pull request as ready for review January 27, 2026 11:21
@lucaslopezf lucaslopezf requested a review from a team as a code owner January 27, 2026 11:21
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/obs-exploration-team (Team:obs-exploration)

@lucaslopezf
Copy link
Copy Markdown
Contributor Author

/oblt-deploy

@lucaslopezf lucaslopezf force-pushed the 249695-traces-discover-enhance-flyouts-behavior branch from 81aed43 to e7ec995 Compare January 27, 2026 15:21
@lucaslopezf
Copy link
Copy Markdown
Contributor Author

/oblt-deploy

@iblancof iblancof self-requested a review January 27, 2026 17:14
Copy link
Copy Markdown
Contributor

@iblancof iblancof left a comment

Choose a reason for hiding this comment

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

Tested locally and it feels way better this way 🚀

}: DocumentDetailFlyoutProps) {
const isSpanType = type === spanFlyoutId;

const spanData = useSpanFlyoutData({
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.

Would it make sense to keep the hooks inside SpanFlyoutContent/LogFlyoutContent, so we avoid calling both here when only one is actually needed?

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.

good question!! Is part of the strategy, I'll add a comment just in case but, both hooks short-circuit with empty strings, so no unnecessary API calls are made. We keep them here because WaterfallFlyout needs some data, for example the loading to render the loading skeleton properly

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.

I’m still unsure about the global architecture here.

For example:
DocumentDetailFlyout expects type because it needs to pass it to WaterfallFlyout, which also expects a type to use as flyoutId (is it really needed like this?). Meanwhile, FlyoutContent is also expecting type, but it’s receiving both spanData and logData anyway. It then decides its content based on type for spans and on logData for logs. It’s a bit confusing, as we’re mixing the source of truth for some of the logic here.

Wdyt about giving it another look?

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.

Good catch! I’ll go part by part so I can explain it and you can check if I’m missing something.

expects a type to use as flyoutId (is it really needed like this?)

id: You’re right, it's done

It then decides its content based on type for spans and on logData for logs. It’s a bit confusing

You're right, it was inconsistent. I've made it consistent now, both cases check type explicitly.

Global architecture

I understand the confusion here. FlyoutContent acts as a wrapper to avoid something like this:

<WaterfallFlyout
  flyoutId={type}
  onCloseFlyout={onCloseFlyout}
  dataView={dataView}
  hit={hit}
  loading={loading}
  title={title}
>
  {hit && isSpanType ? (
    <SpanFlyoutContent hit={hit} dataView={dataView} activeSection={activeSection} />
  ) : null}
  {hit && !isSpanType && logData.logDataView ? (
    <LogFlyoutContent
      hit={hit}
      logDataView={logData.logDataView}
      error={logData.error}
    />
  ) : null}
</WaterfallFlyout>

imho, that approach is more confusing. With FlyoutContent, we wrap the rendering logic in a more controlled way.

Summary up the overall approach:

Why both hooks are called + FlyoutContent exists:

  • DocumentDetailFlyout acts as a unified entry point to prevent flyout unmount/remount when switching document types (e.g., clicking an error badge from a span to view a log). That's why we need a "God" component.
  • Both hooks are called (with short-circuit for the inactive one) because we need hit, loading, and title for WaterfallFlyout regardless of type.
  • FlyoutContent encapsulates the rendering decision in one place, we can do inline as well is just a thing of criteria.

Besides that, I created the useDocumentFlyoutData hook to consolidate the component’s business logic in one place. Let me know if it’s easier to understand this way. I’m also open to other suggestions if you see a cleaner approach with the same result.

@lucaslopezf lucaslopezf requested a review from iblancof January 28, 2026 17:03
@elastic-vault-github-plugin-prod elastic-vault-github-plugin-prod bot requested a review from a team as a code owner January 29, 2026 11:58
@lucaslopezf lucaslopezf force-pushed the 249695-traces-discover-enhance-flyouts-behavior branch from f4e5c8d to 8575e1e Compare January 29, 2026 14:48
@lucaslopezf lucaslopezf removed the request for review from a team January 29, 2026 14:49
Copy link
Copy Markdown
Contributor

@iblancof iblancof left a comment

Choose a reason for hiding this comment

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

Thanks for applying some of the suggested changes!

I agree with this comment and it still needs to be addressed, but I’m leaving the approval so this can be merged once it’s resolved.

@lucaslopezf lucaslopezf force-pushed the 249695-traces-discover-enhance-flyouts-behavior branch from 8575e1e to 51c5c3c Compare January 30, 2026 09:13
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
unifiedDocViewer 792 796 +4

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
unifiedDocViewer 381.3KB 382.1KB +811.0B

History

Copy link
Copy Markdown
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM

@lucaslopezf lucaslopezf merged commit a64684b into elastic:main Jan 30, 2026
16 checks passed
hannahbrooks pushed a commit to hannahbrooks/kibana that referenced this pull request Jan 30, 2026
…es in Trace Waterfall (elastic#250406)

Closes: elastic#249695

## Summary

Previously, the trace waterfall had two separate flyout components for
spans and logs. While this design was architecturally valid, it caused a
suboptimal user experience: when switching between document types (e.g.,
clicking an error badge to view a log), the flyout would unmount and
remount, causing a visual flash and unnecessary loading states.

## Solution: Unified DocumentDetailFlyout

We introduced `DocumentDetailFlyout` as a single entry point that
internally delegates to the appropriate content component based on the
document type.

Note: These components are designed specifically for the trace waterfall
context and are not intended for reuse elsewhere. If standalone document
flyouts are needed in the future, a more generalized abstraction can be
considered at that time.

```
┌─────────────────────────────────────────────────────────────┐
│                    FullScreenWaterfall                       │
│  (manages docId, docIndex, activeFlyoutType, activeSection) │
└─────────────────────────┬───────────────────────────────────┘
                          │
                          ▼
┌─────────────────────────────────────────────────────────────┐
│                   DocumentDetailFlyout                       │
│  - Discriminates by `type` (spanFlyoutId | logsFlyoutId)    │
│  - Calls appropriate data hook                               │
│  - Renders WaterfallFlyout wrapper                          │
└─────────────────────────┬───────────────────────────────────┘
                          │
          ┌───────────────┴───────────────┐
          ▼                               ▼
┌─────────────────────┐       ┌─────────────────────┐
│   SpanFlyoutContent │       │   LogFlyoutContent  │
└─────────────────────┘       └─────────────────────┘
```

## Key Architectural Decisions

### 1. Hook + Content Separation

Each flyout type is split into:
- **Data hook** (`useSpanFlyoutData`, `useLogFlyoutData`) - fetches and
transforms data
- **Content component** (`SpanFlyoutContent`, `LogFlyoutContent`) -
renders the Overview tab

**Why?**
- Allows `DocumentDetailFlyout` to call both hooks and select the
appropriate data
- Content components receive only what they need (no null checks
internally)
- Easier to test data fetching separately from rendering

## Usage from Parent

The parent component (`FullScreenWaterfall`) only needs to:

1. Track the document ID and active flyout type
2. Optionally track the document index (for logs) and active section
(for span scroll-to behavior)
3. Pass the trace dataView for the Table/JSON tabs

The unified flyout handles all the complexity of fetching data and
rendering the appropriate content.

## Demo


https://github.com/user-attachments/assets/c0d8124d-b006-4199-9cfb-933284fb1ffa


### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
- [ ] Review the [backport
guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)
and apply applicable `backport:*` labels.

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] [See some risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
- [ ] ...
mbondyra added a commit to mbondyra/kibana that referenced this pull request Feb 2, 2026
…iew_cps

* commit '32efd9b2fb078ade51073fd2d0068bc74c029d6b': (49 commits)
  [Security Solution] Rules exceptions subfeatures (elastic#245722)
  [BK] Upgrade axios (elastic#251150)
  Fix AI Connector form fields resetting to default value when cleared by user (elastic#251095)
  deduplicate otel dependencies (elastic#250841)
  Adds initial agents.md file (elastic#250833)
  [index management] Faster index list loading (elastic#246276)
  skip failing test suite (elastic#251086)
  skip failing test suite (elastic#251048)
  [Security Solutions] Trial Companion - adjust UX design (elastic#250910)
  [Traces][Discover] Prevent flyout remount when switching document types in Trace Waterfall (elastic#250406)
  [DOCS][Cases][9.4 & Serverless]: Doc new `Maximum amount of cases to open` setting for case action (elastic#250993)
  [Discover][Traces] Explore trace.id from logs in Discover (elastic#249632)
  Remove ! from SOs docs link (elastic#251097)
  [ML] Maps: Add telemetry events for file uploads (elastic#247543)
  [Fleet] Fix dupplicate ids when copying an integration policy or an agent policy (elastic#250971)
  [Dashboards as Code] Add snake case object keys util (elastic#250962)
  [Core] Remove URL Overflow & Deprecate `storeInSessionStorage` setting (elastic#242972)
  [One Workflow] fix: Fix Variable Retrieval in Workflow Execution Engine (elastic#250852)
  Rework Elastic Managed LLMs page (elastic#251069)
  [Lens powered by ES|QL] Update Switch to Query mode modal warning message (elastic#251051)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:enhancement Team:obs-exploration Observability Exploration team v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Traces][Discover] Flyout remounts when switching between Span and Log document types

4 participants