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: use new buildTree utility function #71

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mattrothenberg
Copy link
Contributor

@mattrothenberg mattrothenberg commented Jan 4, 2023

Companion PR to githubnext/blocks-dev#21.

This PR fixes NEXT-634.

There's still an open experiential (and technical question) as to whether the file picker should disclose all of the files in the repository, or only those that are children of the current tree node.

In order to implement the former, I think we'll need an upstream change in blocks-platform so that the tree prop that gets passed down to folder blocks is always the highest-level tree, rather than the branch that's sliced according to the current path.

A pre-requisite for merging this is publishing the @githubnext/blocks library to the main channel again. Currently, I'm using a next build.

@mattrothenberg mattrothenberg requested review from Wattenberger and jaked and removed request for Wattenberger January 4, 2023 08:16
Comment on lines -14 to -15
// needed to close the dropdown, which is an uncontrolled detail element
const [iteration, setIteration] = useState(0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wasn't necessary on my end, though I might be missing some context or was testing it incorrectly. The dropdown closes fine upon item click; this behavior seems to be the default, looking at the Primer docs.

Copy link

@jaked jaked left a comment

Choose a reason for hiding this comment

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

import {
Block,
FolderBlockProps,
getNestedFileTree,
Copy link

Choose a reason for hiding this comment

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

is getNestedFileTree unused now?

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 believe so! Will fix :)

@@ -22,7 +22,7 @@
"@excalidraw/excalidraw": "^0.10.0",
"@fullstackio/cq": "^6.0.9",
"@fullstackio/remark-cq": "^6.1.2",
"@githubnext/blocks": "^2.2.0",
"@githubnext/blocks": "^2.4.0-1",
Copy link

Choose a reason for hiding this comment

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

how do you make a candidate release like this?

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 use the tool np which allows you to create pre-releases and publish them to the next tag on NPM.

@jaked
Copy link

jaked commented Jan 6, 2023

another thought here: should we consider changing the Blocks API so file trees are passed in a more useful way? rather than needing a library function in the block

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