-
Notifications
You must be signed in to change notification settings - Fork 59
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: implement the FileInput API for extra.FileInput #288
base: master
Are you sure you want to change the base?
Conversation
Also I made a reacton component for testing this code based on import threading
from typing import Callable, TypedDict, BinaryIO, cast
import reacton
import solara
from ipyvuetify.extra.file_input import FileInput
class FileInfo(TypedDict):
name: str
size: int
lastModified: int
type: str
file_obj: BinaryIO
data: bytes | None
@reacton.component
def Upload(on_upload: Callable[[list[FileInfo]], None] = lambda _: None, lazy: bool = False, **kwargs):
file_info, set_file_info = reacton.use_state(None)
wired_files, set_wired_files = reacton.use_state(cast(list[FileInfo] | None, None))
file_input = FileInput.element(
outlined=True,
counter=True,
multiple=True,
show_size=True,
label="Files",
placeholder="Select files...",
chips=True,
color="purple",
on_v_model=set_file_info,
disabled=bool(file_info),
loading=bool(file_info),
prepend_icon="mdi-upload",
attributes={"accept": "image/*"},
**kwargs
)
def wire_files():
if not file_info:
return
real = cast(FileInput, reacton.get_widget(file_input))
# workaround for @observe being cleared
real.version += 1
real.reset_stats()
set_wired_files(cast(list[FileInfo], real.get_files()))
reacton.use_effect(wire_files, [file_info])
def handle_file(cancel: threading.Event):
if not wired_files:
return
if on_upload:
for wired_file in wired_files:
wired_file["data"] = None if lazy else wired_file["file_obj"].read()
on_upload(wired_files)
set_file_info(None)
result: solara.Result = solara.hooks.use_thread(handle_file, [wired_files])
if result.error:
raise result.error
return file_input
if __name__ == "__main__":
@reacton.component
def Page():
Upload(on_upload=print) |
c7b0e3d
to
2c5cbb4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really ❤️ warming to see you spending effort on being backwards compatible!
I really love this PR. I do wonder if the deprecation warnings should be emitted now, as that can be seen as a breaking change? Maybe be silent now, and for ipyvue v3 we do warn the user?
What do you think Mario?
ipyvuetify/extra/file_input.py
Outdated
del kwargs["v_model"] | ||
|
||
# Maintain backwards compatibility for the file_info | ||
dlink((self, "v_model"), (self, "file_info")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is better done on the python side. If we have file_info
not have sync=True, and observe v_model, and set file_info as a result of that, we don't have to transfer the data twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already removed sync tag for file_info
. It seems that dlink
does what you described in this case: it updates the target value once on initialization and adds the observer, which modifies the target value on change to the source value.
Yeah, this is great! I didn't know you could override a slot by adding it again. I also overlooked there was a slot for progress. I agree with @maartenbreddels that we should not show warnings for the changed properties. In the notebook these will show up for the end-user. That end-user may not be the author and may not know how to fix it. Looks like only Note: I will force push to this branch to remove the erroneous commit that was temporarily on master. |
7094c9b
to
4ae2717
Compare
I see there are lint-errors. This can be prevented by using pre-commit. I've just updated the readme with these instructions: $ pip install -e ".[dev]"
$ pre-commit install This will check and enforce code style when committing. To update the existing code you can run: $ pre-commit run --all-files |
I've removed warnings and fixed code style issues. I have a question: why this workaround is needed for solara.FileDrop? How observe can be cleared? real.version += 1
real.reset_stats() UPD: |
I added a computed property as a default I also thought about transforming v_model values into objects, that use pythonic key names (last_modified instead of lastModified) and maybe even allow attribute access with dot notation (namedtuple?). It should be easier to do now, before this PR is merged. What do you think? UPD: I've almost finished adding drag & drop 😃 |
Sorry for the late response, I missed the notifications for the changes on this PR.
Wouldn't it be better to leave out The drag & drop functionality is really nice!, but it shouldn't be part of this PR. Are you able to move those commits to a new PR (that can be based on this PR)? |
Hi, sorry to ask but is there any plan to merge this PR this at some point? Should it wait for a Vuetify3 upgrade? Thanks ;) |
Hi!
I noticed that the FileInput from
ipyvuetify.extra
doesn't support all properties of v-file-input. In this PR I tried to implement most of the FileInput API while maintaining backward compatibility where possible. I suppose it closes #265.I haven't found a beautiful way to pass v-file-input props to component without hardcoding the property names. I tried to filter keys of
this.$data
, but that will lead to possible issues with components derived fromFileInput
, such assolara.FileDrop
. So I think this implementation will do for now, and the further task will be to combine it with the generatedFileInput
.Please let me know your thoughts about this.