action_card: display output dirs as trees #6545
Conversation
sluongng
force-pushed
the
sluongng/improve-action-card
branch
from
cfd83d0
to
d255d63
Compare
sluongng
force-pushed
the
sluongng/improve-action-card
branch
from
d255d63
to
b325623
Compare
| obj: new build.bazel.remote.execution.v2.DirectoryNode({ name: dir.path, digest: dir.treeDigest }), | ||
| type: "dir", | ||
| }} | ||
| treeShaToExpanded={this.state.treeShaToExpanded} |
There was a problem hiding this comment.
Unlike input, which only has one tree, there could be multiple output directories in ActionResult, which is currently translated to multiple trees. This has aside effects:
The current implementation shares the same treeShaToExpanded state among all input/output trees. This could cause expanding 1 tree in input to also expand that tree in output. In practice, I think it's unlikely that these directories are duplicated. We could fix this later by providing a separate state for each tree.
There was a problem hiding this comment.
A second problem with this approach is that if there are 2 or more output dirs, the prefix bazel-out/<target-platform>/bin prefix is not duplicated between the rendered trees.
I briefly considered deduplicating them, but when we do that, it would not be clear to the user which directories in the merged tree were the output directory specified in the ActionResult.
I plan to add Expected output paths to render the Command data in a future PR. Once that's done, I think we could reconsider merging the output trees in our UI.
sluongng
force-pushed
the
sluongng/improve-action-card
branch
from
b325623
to
b17c56f
Compare
|
A preview
|
Some style nits: It looks like the two new icons need to be sized the same as the folder / download icons - and I suspect some Also, is there a digest for these we could render like for the other files? |
Render the output directories as expandable trees so that users could explore their contents. Also show output symlinks.
sluongng
force-pushed
the
sluongng/improve-action-card
branch
from
b17c56f
to
b34b04a
Compare
|
Here is the latest preview which includes @bduffany CSS fix from the previous PR
The symlink could point to an existing file in the tree, or it could point to a file on the execution host or a non-existing file, in which case it would be a dangling symlink. We could consider adding a digest when the file it points to does exist in the tree, but I would prefer attempting that in a separate PR. |
The new icons look a little darker than the download / file icons. And I feel like the arrow icon should potentially be a bit smaller. |
Changed the opacity of the symlink icons to be more consistent with the rest of the icons in the tree. |
sluongng
force-pushed
the
sluongng/improve-action-card
branch
from
6b87d90
to
f7dc856
Compare
Render the output directories as expandable trees so that users could
explore their contents.
This PR is based on #6525 so we should only merge this PR after.