Skip to content

Jdb/poc-inline-insights

Warren Gifford requested to merge jdb/poc-inline-insights into main

Created by: unclejustin

Closes https://github.com/sourcegraph/sourcegraph/issues/39112

Related BE PR: https://github.com/sourcegraph/sourcegraph/pull/39475

To set up pick any file. I've been using this one: https://sourcegraph.test:3443/github.com/sourcegraph/sourcegraph/-/blob/enterprise/internal/insights/query/querybuilder/builder.go?L24

Then create an insight with any keyword in that file. You should see the line decoration for that file. Clicking the decoration should open the popover. Clicking it again should close the popover.

Known Issues:

  • Links aren't clickable. Popover seems to render beneath code.
  • To keep the popover from closing when mousing away from the decoration, I set hideOnClick="toggle" in LineDecorator. This works but also means the popover doesn't close when you click outside the popover. Which is not great.

Remaining Items:

  • Fix the above. Probably clean up how we pull data. (I am so sorry)
  • Build the dropdown for the file at the top of the blob view. You should be able to reuse the popover component I already built.
  • Extract all of the logic building the decorations in Blob.tsx to a utility so that we don't clutter up Blob anymore.
  • Make sure all of this is feature flagged default off

Note

Felix recommended breaking this up into 3 separate issues. I've stubbed them out below. I would take the code in this PR and pull what you need into brand new feature branches to build these one at a time.

For example: Grab the popover component and build the file drop-down first.

Here are the issues in order of what makes sense so that they build on each other:

  1. https://github.com/sourcegraph/sourcegraph/issues/39550
  2. https://github.com/sourcegraph/sourcegraph/issues/39551
  3. https://github.com/sourcegraph/sourcegraph/issues/39552

Note 2

@Joelkw actually wants item 3 above done first, but it doesn't make sense to build it this way. (Sorry Joel). I recommended breaking this up into smaller pieces and getting them reviewed/merged separately.

Test plan

N/A This is just a spike

App preview:

Check out the client app preview documentation to learn more.

Merge request reports