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

New Object Inspector UI (architecture investigation) #7235

Merged
merged 20 commits into from
Jun 21, 2022

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Jun 21, 2022

This PR adds a new Object Inspector. Key concepts include:

  • Object Inspector re-written as declarative components.
  • Formatting of values matches a mix of Chrome and Firefox (bit more of Chrome).
  • Object preview data is lazily fetched and cached using React Suspense.
    • This totally removes dependency on ValueFront.
    • I also explored a new format for our object previews (see BAC-1808 for more).
  • New React Offscreen API used to remember state (e.g. collapsed/expanded) in conditionally-rendered children.

Loom walk through of the architecture:
https://www.loom.com/share/69b18fb36bfb4ab6b70a2bda49afa499

To test the new component:

  1. Pull down this branch
  2. Run the Next test shell (cd packages/bvaughn-architecture-demo && npm run dev)
  3. Open a recording, e.g. localhost:3000/?recordingId=4e6e2745-13ef-49ae-9aee-dad6d8a71deb

FE-322

@vercel
Copy link

vercel bot commented Jun 21, 2022

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

Name Status Preview Updated
devtools ✅ Ready (Inspect) Visit Preview Jun 21, 2022 at 6:03PM (UTC)

@markerikson
Copy link
Collaborator

Obviously there's a lot of code here :)

Skimmed through it, and it seems pretty legible, well-structured, and well-commented.

Really, my main question is: what's a feasible path to adding this to the existing codebase?

export interface GetTestsRun_node_Workspace_testRuns_recordings_edges_node {
__typename: "Recording";
uuid: any;
duration: number | null;
createdAt: any;
metadata: any | null;
comments: GetTestsRun_node_Workspace_testRuns_recordings_edges_node_comments[];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wish I knew why I had to update this file to make our CI type check happy. I didn't touch anything related to this.

@bvaughn
Copy link
Contributor Author

bvaughn commented Jun 21, 2022

Thanks Mark!

After some discussion over VC, I'm going to go ahead and land this change set, and then iterate on it some more with a follow up PR.

@bvaughn bvaughn merged commit 5f1aca7 into main Jun 21, 2022
@bvaughn bvaughn deleted the bvaughn/architecture-pt3 branch June 21, 2022 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants