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.

Sign up for GitHub

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

FilePickerControl features like new event and fixes upload behavior between page update #3875

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

FilePickerControl features like new event and fixes upload behavior between page update #3875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 40 additions & 35 deletions packages/flet/lib/src/controls/file_picker.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:http/http.dart' as http;

import '../flet_app_services.dart';
import '../flet_control_backend.dart';
import '../flet_server.dart';
import '../models/control.dart';
import '../utils/desktop.dart';
import '../utils/strings.dart';
Expand Down Expand Up @@ -78,12 +76,12 @@ class FilePickerControl extends StatefulWidget {
State<FilePickerControl> createState() => _FilePickerControlState();
}

List<PlatformFile> files = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Consider moving the 'files' list into the state class

Global variables can lead to unexpected behavior and make the code harder to maintain. It would be better to keep this list as part of the _FilePickerControlState class.

Suggested change
List<PlatformFile> files = [];
class _FilePickerControlState extends State<FilePickerControl> {
List<PlatformFile> files = [];
// Rest of the class implementation...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the state class, the files variable becomes unstable during page refreshes.

Copy link
Contributor

Choose a reason for hiding this comment

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

It sounds like the instability of the files variable during page refreshes might be due to how the state is being managed or preserved across refreshes. One approach to address this could be to ensure that the state is properly initialized or restored when the component is rebuilt.

You might want to consider using a state management solution or a mechanism to persist the state across refreshes, such as using a StatefulWidget with a State that properly handles initialization and disposal. Additionally, you could explore using local storage or a similar method to save and restore the files list if it needs to persist across page reloads.

If you can provide more details about how the state is currently being managed, I might be able to offer more specific suggestions.


class _FilePickerControlState extends State<FilePickerControl>
with FletStoreMixin {
String? _state;
String? _upload;
String? _path;
List<PlatformFile>? _files;

@override
Widget build(BuildContext context) {
Expand All @@ -108,11 +106,8 @@ class _FilePickerControlState extends State<FilePickerControl>
}

debugPrint("FilePicker _state: $_state, state: $state");

resetDialogState() {
_state = null;
widget.backend.updateControlState(widget.control.id, {"state": ""});
}
debugPrint("FilePicker _path: $_path");
debugPrint("FilePicker _files: $files");

sendEvent() {
if (defaultTargetPlatform != TargetPlatform.windows || !isDesktop()) {
Expand All @@ -123,8 +118,8 @@ class _FilePickerControlState extends State<FilePickerControl>
"result",
json.encode(FilePickerResultEvent(
path: _path,
files: _files
?.map((f) => FilePickerFile(
files: files
.map((f) => FilePickerFile(
name: f.name,
path: kIsWeb ? null : f.path,
size: f.size))
Expand All @@ -133,9 +128,8 @@ class _FilePickerControlState extends State<FilePickerControl>

if (_state != state) {
_path = null;
_files = null;
// _files = null;
FeodorFitsner marked this conversation as resolved.
Show resolved Hide resolved
_state = state;

if (isDesktop() && defaultTargetPlatform == TargetPlatform.windows) {
resetDialogState();
}
Expand All @@ -154,7 +148,10 @@ class _FilePickerControlState extends State<FilePickerControl>
withReadStream: true)
.then((result) {
debugPrint("pickFiles() completed");
_files = result?.files;

if (result != null) {
files = result.files;
}
sendEvent();
});
}
Expand Down Expand Up @@ -188,40 +185,39 @@ class _FilePickerControlState extends State<FilePickerControl>
_path = result;
sendEvent();
});
} else if (state?.toLowerCase() == "uploadfiles" &&
upload != null &&
files.isNotEmpty) {
uploadFiles(upload, files, pageArgs.pageUri!);
}
}

// upload files
if (_upload != upload && upload != null && _files != null) {
_upload = upload;
uploadFiles(
upload, FletAppServices.of(context).server, pageArgs.pageUri!);
}

return widget.nextChild ?? const SizedBox.shrink();
});
}

Future uploadFiles(String filesJson, FletServer server, Uri pageUri) async {
Future uploadFiles(
String filesJson, List<PlatformFile> files, Uri pageUri) async {
var uj = json.decode(filesJson);
var uploadFiles = (uj as List).map((u) => FilePickerUploadFile(
name: u["name"], uploadUrl: u["upload_url"], method: u["method"]));
List<String> uploadedFiles = [];
for (var uf in uploadFiles) {
var file = _files!.firstWhereOrNull((f) => f.name == uf.name);
var file = files.firstWhereOrNull((f) => f.name == uf.name);
if (file != null) {
try {
await uploadFile(
file, server, getFullUploadUrl(pageUri, uf.uploadUrl), uf.method);
_files!.remove(file);
file, getFullUploadUrl(pageUri, uf.uploadUrl), uf.method);
uploadedFiles.add(file.name);
} catch (e) {
sendProgress(server, file.name, null, e.toString());
sendProgress(file.name, null, e.toString());
}
}
}
sendUploadFinishEvent(uploadedFiles);
resetDialogState();
}

Future uploadFile(PlatformFile file, FletServer server, String uploadUrl,
String method) async {
Future uploadFile(PlatformFile file, String uploadUrl, String method) async {
final fileReadStream = file.readStream;
if (fileReadStream == null) {
throw Exception('Cannot read file from null stream');
Expand All @@ -234,7 +230,7 @@ class _FilePickerControlState extends State<FilePickerControl>
streamedRequest.contentLength = file.size;

// send 0%
sendProgress(server, file.name, 0, null);
sendProgress(file.name, 0, null);

double lastSent = 0; // send every 10%
double progress = 0;
Expand All @@ -247,7 +243,7 @@ class _FilePickerControlState extends State<FilePickerControl>
if (progress >= lastSent) {
lastSent += 0.1;
if (progress != 1.0) {
sendProgress(server, file.name, progress, null);
sendProgress(file.name, progress, null);
}
}
}, onDone: () {
Expand All @@ -257,23 +253,32 @@ class _FilePickerControlState extends State<FilePickerControl>
var streamedResponse = await streamedRequest.send();
var response = await http.Response.fromStream(streamedResponse);
if (response.statusCode < 200 || response.statusCode > 204) {
sendProgress(server, file.name, null,
sendProgress(file.name, null,
"Upload endpoint returned code ${response.statusCode}: ${response.body}");
} else {
// send 100%
sendProgress(server, file.name, progress, null);
sendProgress(file.name, progress, null);
}
}

void sendProgress(
FletServer server, String name, double? progress, String? error) {
resetDialogState() {
_state = null;
widget.backend.updateControlState(widget.control.id, {"state": ""});
}

void sendProgress(String name, double? progress, String? error) {
widget.backend.triggerControlEvent(
widget.control.id,
"upload",
json.encode(FilePickerUploadProgressEvent(
name: name, progress: progress, error: error)));
}

void sendUploadFinishEvent(List<String> files) {
widget.backend.triggerControlEvent(
widget.control.id, "upload_finished", json.encode(files));
}

String getFullUploadUrl(Uri pageUri, String uploadUrl) {
Uri uploadUri = Uri.parse(uploadUrl);
if (!uploadUri.hasAuthority) {
Expand Down
18 changes: 18 additions & 0 deletions sdk/python/packages/flet-core/src/flet_core/file_picker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class FilePickerState(Enum):
PICK_FILES = "pickFiles"
SAVE_FILE = "saveFile"
GET_DIRECTORY_PATH = "getDirectoryPath"
UPLOADING_FILES = "uploadfiles"


class FilePickerFileType(Enum):
Expand Down Expand Up @@ -114,6 +115,7 @@ def __init__(
self,
on_result: Optional[Callable[[FilePickerResultEvent], None]] = None,
on_upload: Optional[Callable[[FilePickerUploadEvent], None]] = None,
on_upload_finished: Optional[Callable[[ControlEvent], None]] = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (complexity): Consider whether the new state and event handler are essential.

The recent changes have increased the complexity of the FilePicker class. The addition of a new state (UPLOADING_FILES) and event handler (on_upload_finished) adds more logic to manage, which can make the code harder to maintain. The on_upload_finished handler seems redundant as a simple pass-through, potentially adding unnecessary complexity. Consider whether these additions are essential. If they are, aim to integrate them with minimal redundancy and straightforward logic. I've refactored the code to ensure on_upload_finished is only added if provided, and encapsulated event data conversion for better readability. This approach reduces complexity and improves maintainability.

#
# Control
#
Expand All @@ -138,13 +140,19 @@ def convert_result_event_data(e):
self.__on_upload = EventHandler(lambda e: FilePickerUploadEvent(e))
self._add_event_handler("upload", self.__on_upload.get_handler())

self.__on_upload_finished = EventHandler(lambda e: e)
self._add_event_handler(
"upload_finished", self.__on_upload_finished.get_handler()
)

self.__result: Optional[FilePickerResultEvent] = None
self.__upload: List[FilePickerUploadFile] = []
self.__allowed_extensions: Optional[List[str]] = None
self.__state = None
self.__file_type = None
self.on_result = on_result
self.on_upload = on_upload
self.on_upload_finished = on_upload_finished

def _get_control_name(self):
return "filepicker"
Expand Down Expand Up @@ -248,6 +256,7 @@ async def get_directory_path_async(

def upload(self, files: List[FilePickerUploadFile]):
self.__upload = files
self.state = FilePickerState.UPLOADING_FILES
self.update()

@deprecated(
Expand Down Expand Up @@ -345,3 +354,12 @@ def on_upload(self):
@on_upload.setter
def on_upload(self, handler: OptionalEventCallable[FilePickerUploadEvent]):
self.__on_upload.handler = handler

# on_upload_finished
@property
def on_upload_finished(self):
return self.__on_upload_finished.handler

@on_upload_finished.setter
def on_upload_finished(self, handler: OptionalEventCallable[ControlEvent]):
self.__on_upload_finished.handler = handler