marimo-team / marimo

A reactive notebook for Python — run reproducible experiments, execute as a script, deploy as an app, and version with git.
https://marimo.io
Apache License 2.0
6.62k stars 225 forks source link

improve: Select folders and select all in File Browser #1558

Closed wasimsandhu closed 4 months ago

wasimsandhu commented 4 months ago

📝 Summary

Enhances the File Browser component (fulfilling #1478) with a few bug fixes.

Fixes #1478

🔍 Description of Changes

Screenshot 2024-06-04 at 10 15 15 PM

📋 Checklist

📜 Reviewers

vercel[bot] commented 4 months ago

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

Name Status Preview Comments Updated (UTC)
marimo-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 6, 2024 6:48am
marimo-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 6, 2024 6:48am
mscolnick commented 4 months ago

Thoughts on make but button more minimal? In the table we use variant="link".

And maybe we can only show one at a time? If we are in a "select all" state, then show "deselect all" instead.

wasimsandhu commented 4 months ago

Screenshot 2024-06-05 at 6 28 29 PM

wasimsandhu commented 4 months ago

Lol did not realize I created a bug until I shared that screenshot.

wasimsandhu commented 4 months ago

First draft is a bit messy, definitely need to do some code and UI cleanup before merging.

Added the selection_mode parameter, which defaults to "file". If set to "directory" or "all", user can check a checkbox next to the folder name to select the folder.

I spent 30 minutes trying to get the checkbox to display nicely to the left of the file icon and just couldn't get the margins right. Ended up defaulting to displaying the checkbox if an item is selected, or always displaying it for selecting directories.

We could display the checkbox on hover, but would managing too many states create a significant performance overhead? @mscolnick

Screenshot 2024-06-05 at 11 49 11 PM

I don't have a good use case for selecting both files and directories off the top of my head, but this gives the user more flexibility I guess.

I also think it might be worth tweaking the printed representation of a FileInfo list (printing b.value is not very user friendly). I'd probably just do a tab-delimited array of each file's name, path, is_directory, and last_modified_date.

mscolnick commented 4 months ago

this looks great! i can take a pass of my own to do the styling

wasimsandhu commented 4 months ago

Thanks, that'd be great!

mscolnick commented 4 months ago

going to merge, and fix from main

github-actions[bot] commented 4 months ago

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.6.17-dev3