Skip to content

Conversation

@Zylphrex
Copy link
Member

This change adds a filter dropdown to the spans page that can be used to filter
the specific span ops. This is limited to the top 20 most frequent span ops at
the moment.

This change adds a filter dropdown to the spans page that can be used to filter
the specific span ops. This is limited to the top 20 most frequent span ops at
the moment.
@Zylphrex Zylphrex requested a review from a team October 28, 2021 17:53
};
}

const spanOp = decodeScalar(location.query.spanOp);
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR, but as an aside, looking at how many of these we end up doing, I feel like we should have a useLocation and then have useQueryField('QueryField.SPAN_OP') eventually, with enums to ensure we don't have typos selecting from the query 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

That makes sense to me, we can follow up with this later.

<Radio
radioSize="small"
checked={spanOp.op === currentOp}
onChange={() => {}}
Copy link
Member

Choose a reason for hiding this comment

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

Hmm I didn't know we would do a fake radio and make the whole row clickable but that's probably better considering a dropdown is ephemeral. I'm guessing we do this elsewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

We actually have this pattern for the ops breakdown filter already. I had to add this empty onChange handler to make a warning go away about the radio button missing a change handler.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it's fine, the empty onChange is what caught my eye, radios can have small click areas anyway.

@Zylphrex Zylphrex merged commit 18bfb8c into master Nov 1, 2021
@Zylphrex Zylphrex deleted the feat/add-ops-filter-to-spans-tab branch November 1, 2021 19:56
@github-actions github-actions bot locked and limited conversation to collaborators Nov 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants