New chart from different dataset

[ptbrowne]

Ability to add a new chart config based on a new dataset

We can add a new chart config based on a different dataset than the one we currently
see.

• Opening of panel
• Selection of dataset
• Search works in panel
• Order works in panel
• Filters work in panel
• Can add dataset

Looking at it, I am not too happy with how the code is organised for that since I need to drill down on click callbacks, I may have to refactor using a context that would

• hold the callbacks
• activate or deactivate links

I'd be happy if you can have a look with this in mind @bprusinowski , if you have an idea ?

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
visualization-tool	✅ Ready (Inspect)	Visit Preview	✅ 2 resolved	May 28, 2024 0:33am

[bprusinowski]

LGTM, works and feels nice 💯

While reviewing the code I didn't have the impression that it's a lot of prop drilling, maybe it's also a part of the code that wasn't modified and I didn't see it?

But generally I think having a context would help with cleaning up a bit – and I think here everything connected to browsing page it closely tied to each other, so introducing shared context shouldn't cause problems, as we probably won't re-use the components outside of browsing page / drawer. So if you felt the struggle, I think it would be a good thing to do 👌

[ptbrowne]

I cleaned up a bit the part about the two different providers, so that from the consumer side (the page or the drawer), they would only change the variant but would not know about the details of URL sync. I also removed the need for different providers, instead I create the correct hook based on the syncWithUrl parameter. I find it more readable, and the low level of details of using useBrowseStateQueryParams or useState are hidden inside createUseBrowseState.

I will not go further in the de-entanglement for now, thanks for the feedback that apparently props drilling was not too much.

I put it there so to be able to come back to it, but I found myself wanting to be able to control the behavior of Links = have a way to "hook" into the next js Link from the context, and react to it.

const SelectPageDrawer = () => {
   const [dataset, setDataset] = useState()
  return <InterceptableLinkContext onClickLink={(ev, url) => {
     if (url.matches('/dataset')) {
         setDataset(getDatasetFromUrl(url))
         ev.preventDefault()
	 }
    }
  }}>
        <SelectPageContent />
   </InterceptableLinkContext>
}

const SelectPageContent = () => {
      return <>
           <Nav />
           <Results />
      </>
}

const Results = () => {
      return results.map(x => {
           return <InterceptableLink href={`/dataset/${x.iri}`}><MuiLink /></InterceptableLink>
       })
}

Is this a good idea I wonder 🤔 .

This way, you would be able to surgically control the behavior of links. You would lose in type safety, whereas with prop drilling, you know exactly what to do, at the expense of verboseness.

[bprusinowski]

Ah, so you'd actually have Links that are Links or Buttons with onClick event, depending on the variant (page vs drawer) – but the components would actually stay the same (in simplification)? I think that's an interesting idea 👍 🥹 And should also be understandable with a small docstring 😅

[ptbrowne]

Yes exactly, because it's good I find to have links with the hover and right click behavior of links inside the page. And also if we can keep the behavior of links for cmd + click and right click > open new tab n the context of drawer, that's good. That's why I want to keep links.