Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add VizPanelExploreButton scene object #804

Merged
merged 4 commits into from
Nov 12, 2024

Conversation

domasx2
Copy link
Contributor

@domasx2 domasx2 commented Jun 26, 2024

Hey,

On popular request, contributing a panel explore button derived from app o11y implementation. It's been copied around to other apps, so seems to make sense to contribute it to scenes.

Does this make sense? If so, I'll add tests. Also thinking it's maybe whorthwile to make a getExploreUrl(sceneObject) util as it's own thing in case you want something else than a button

panel explore button

Comment on lines 30 to 56
function VizPanelExploreButtonComponent({ model }: SceneComponentProps<VizPanelExploreButton>) {
const { options } = model.useState();

const { data } = sceneGraph.getData(model).useState();

const targets = useMemo(() => data?.request?.targets ?? [], [data]);

const { from, to } = sceneGraph.getTimeRange(model).useState();

const { value: interpolatedQueries } = useAsync(async () => {
const scopedVars: ScopedVars = {
__sceneObject: { text: '__sceneObject', value: model },
};
return (
await Promise.allSettled(
targets.map(async (q) => {
const queryDs = await getDataSourceSrv().get(q.datasource);
return queryDs.interpolateVariablesInQueries?.([q], scopedVars ?? {})[0] || q;
})
)
)
.filter((promise): promise is PromiseFulfilledResult<DataQuery> => promise.status === 'fulfilled')
.map((q) => q.value)
.map((q) => options.transform?.(q) ?? q);
}, [targets, model]);

const left = useMemo(() => {
Copy link
Member

Choose a reason for hiding this comment

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

I usually prefer plain links vs buttons with onClick , but feels a bit unnecessary to do all this work on every data refresh (and re-render Linkbutton), vs doing onClick. Maybe building the URL onHover and onFocus ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this button is a link styled as a button, it does have proper href. the onClick is an optional callback for adding tracking of clicks via Faro/Rudderstack if needed. However I'm now not sure it will react to variable updates properly, needs investigation 🤔 maybe onHover is a good idea

@domasx2 domasx2 closed this Oct 10, 2024
@torkelo
Copy link
Member

torkelo commented Oct 10, 2024

@domasx2 why did you close, think this is a great addition.

sorry for not approving it, it must have slipped of my radar / review list

@domasx2
Copy link
Contributor Author

domasx2 commented Oct 10, 2024

I realized this has been hanging for a long while, and thought if I didn't find time and energy to push it, best close it. Do you think with some tests added this would be ok to merge, or needs more work?

@domasx2 domasx2 reopened this Oct 10, 2024
@domasx2 domasx2 force-pushed the domas-add-viz-panel-explore-button branch from 72db51e to 2c63320 Compare November 11, 2024 12:54
@domasx2
Copy link
Contributor Author

domasx2 commented Nov 11, 2024

extracted "getExploreURL" as separate util for use cases outside of button component, added "return to previous" integration, made it ad hoc filters aware

@domasx2 domasx2 requested a review from torkelo November 11, 2024 14:46
Copy link
Member

@torkelo torkelo left a comment

Choose a reason for hiding this comment

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

nice!

Maybe post about this one in #proj-scene-apps

@domasx2 domasx2 merged commit afa4af9 into main Nov 12, 2024
5 checks passed
@domasx2 domasx2 deleted the domas-add-viz-panel-explore-button branch November 12, 2024 07:34
@scenes-repo-bot-access-token
Copy link

🚀 PR was released in v5.24.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants