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

feat: introduce car explorer (CAR import) #82

Merged
merged 7 commits into from
Apr 13, 2022
Merged

Conversation

alvin-reyes
Copy link
Collaborator

@alvin-reyes alvin-reyes commented Apr 12, 2022

Changes

This change introduce the <IpldCarExplorerForm /> on the explorer.ipld.io.

Related PR

ipfs/ipld-explorer-components#313

Verification

image
image
image

lidel
lidel previously requested changes Apr 13, 2022
Copy link
Collaborator

@lidel lidel left a comment

Choose a reason for hiding this comment

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

LGTM, switched to ipld-explorer-components@2.1.0 and cleaned up some locale deps.

Things to fix before we merge:

  • CAR import does not work when in-memory js-ipfs (ipfs-core NPM dependency) is used
  • UX of selecting a CAR:
    • when CAR file is selected, nothing happens. Took me a moment to realize I need to click "Explore"
    • this is confusing, no need for this additional click – we should open the root CID as soon dag.import is finished.

src/components/header/header.css Outdated Show resolved Hide resolved
@alvin-reyes
Copy link
Collaborator Author

The issue here will be resolved by PR ipfs/ipld-explorer-components#317

@alvin-reyes alvin-reyes marked this pull request as ready for review April 13, 2022 22:06
@alvin-reyes alvin-reyes requested a review from lidel April 13, 2022 22:06
@alvin-reyes alvin-reyes dismissed lidel’s stale review April 13, 2022 22:07

Changes are now applied based on the review points and feedbacks.

Copy link
Collaborator

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Works as expected now 👍

Let's merge, we can improve UI in separate PR.

src/components/header/Header.js Outdated Show resolved Hide resolved
@lidel lidel merged commit 84d7b18 into master Apr 13, 2022
@lidel lidel deleted the feat/car-explorer branch April 13, 2022 22:40
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