Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions packages/file_selector/file_selector/lib/file_selector.dart
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ Future<String?> getSavePath({
String? suggestedName,
String? confirmButtonText,
}) async {
// TODO(stuartmorgan): Update this to getSaveLocation in the next federated
// change PR.
// ignore: deprecated_member_use
return FileSelectorPlatform.instance.getSavePath(
acceptedTypeGroups: acceptedTypeGroups,
initialDirectory: initialDirectory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ class SaveTextPage extends StatelessWidget {

Future<void> _saveFile() async {
final String fileName = _nameController.text;
// TODO(stuartmorgan): Update this to getSaveLocation in the next federated
// change PR.
// ignore: deprecated_member_use
final String? path = await FileSelectorPlatform.instance.getSavePath(
// Operation was canceled by the user.
suggestedName: fileName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ class SaveTextPage extends StatelessWidget {

Future<void> _saveFile() async {
final String fileName = _nameController.text;
// TODO(stuartmorgan): Update this to getSaveLocation in the next federated
// change PR.
// ignore: deprecated_member_use
final String? path = await FileSelectorPlatform.instance.getSavePath(
suggestedName: fileName,
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.6.0

* Adds `getSaveLocation` and deprecates `getSavePath`.
Copy link
Member

Choose a reason for hiding this comment

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

If this were a major version bump, wouldn't it make this PR much smaller, without really affecting much further ones down the line? (next you'll implement and use the new method, and remove the ignores, but wouldn't it be ~the same amount of work to bump the minimum version of the dependency of this package in the others?)

Copy link
Member

Choose a reason for hiding this comment

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

(Also, do the "ignores" need to be published to the changed packages?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Major version bumps to platform interfaces are unpleasant if there are any third-party implementations, and enabling those is one of the main goals of federation, so we try to minimize them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(Also, do the "ignores" need to be published to the changed packages?)

Nope, ignores are dev-only changes. I need to teach the tooling to do line-level analysis for the version and changelog checks, and ignore non-doc comments.

Copy link
Member

Choose a reason for hiding this comment

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

Major version bumps to platform interfaces are unpleasant if there are any third-party implementations, and enabling those is one of the main goals of federation, so we try to minimize them.

As an alternative, can you deprecate getSavePath as a last change to the platform interface, once that every package has been updated to use the new version? Otherwise we're going to be yelling at users without them having an available solution, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As an alternative, can you deprecate getSavePath as a last change to the platform interface, once that every package has been updated to use the new version?

Our previous workflow was to in theory do it at the end, but in practice we just always forget and nothing gets deprecated :( And we don't even always remember to update all of our own code to use the new methods. So we're trying this flow instead to see how it goes.

Otherwise we're going to be yelling at users without them having an available solution, right?

Only users who are running analysis on our packages, which I would expect to be nobody.

Copy link
Member

Choose a reason for hiding this comment

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

Our previous workflow was to in theory do it at the end, but in practice we just always forget and nothing gets deprecated :( And we don't even always remember to update all of our own code to use the new methods. So we're trying this flow instead to see how it goes.

git push --force then! :P


## 2.5.1

* Adds compatibility with `http` 1.0.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:async';

import 'package:plugin_platform_interface/plugin_platform_interface.dart';

import '../../file_selector_platform_interface.dart';
Expand Down Expand Up @@ -37,7 +35,10 @@ abstract class FileSelectorPlatform extends PlatformInterface {
}

/// Opens a file dialog for loading files and returns a file path.
/// Returns `null` if user cancels the operation.
///
/// Returns `null` if the user cancels the operation.
// TODO(stuartmorgan): Switch to FileDialogOptions if we ever need to
// duplicate this to add a parameter.
Future<XFile?> openFile({
List<XTypeGroup>? acceptedTypeGroups,
String? initialDirectory,
Expand All @@ -47,6 +48,10 @@ abstract class FileSelectorPlatform extends PlatformInterface {
}

/// Opens a file dialog for loading files and returns a list of file paths.
///
/// Returns an empty list if the user cancels the operation.
// TODO(stuartmorgan): Switch to FileDialogOptions if we ever need to
// duplicate this to add a parameter.
Future<List<XFile>> openFiles({
List<XTypeGroup>? acceptedTypeGroups,
String? initialDirectory,
Expand All @@ -55,8 +60,13 @@ abstract class FileSelectorPlatform extends PlatformInterface {
throw UnimplementedError('openFiles() has not been implemented.');
}

/// Opens a file dialog for saving files and returns a file path at which to save.
/// Returns `null` if user cancels the operation.
/// Opens a file dialog for saving files and returns a file path at which to
/// save.
///
/// Returns `null` if the user cancels the operation.
// TODO(stuartmorgan): Switch to FileDialogOptions if we ever need to
// duplicate this to add a parameter.
@Deprecated('Use getSaveLocation instead')
Future<String?> getSavePath({
List<XTypeGroup>? acceptedTypeGroups,
String? initialDirectory,
Expand All @@ -66,16 +76,41 @@ abstract class FileSelectorPlatform extends PlatformInterface {
throw UnimplementedError('getSavePath() has not been implemented.');
}

/// Opens a file dialog for saving files and returns a file location at which
/// to save.
///
/// Returns `null` if the user cancels the operation.
Future<FileSaveLocation?> getSaveLocation({
List<XTypeGroup>? acceptedTypeGroups,
SaveDialogOptions options = const SaveDialogOptions(),
}) async {
final String? path = await getSavePath(
acceptedTypeGroups: acceptedTypeGroups,
initialDirectory: options.initialDirectory,
suggestedName: options.suggestedName,
confirmButtonText: options.confirmButtonText,
);
return path == null ? null : FileSaveLocation(path);
}

/// Opens a file dialog for loading directories and returns a directory path.
/// Returns `null` if user cancels the operation.
///
/// Returns `null` if the user cancels the operation.
// TODO(stuartmorgan): Switch to FileDialogOptions if we ever need to
// duplicate this to add a parameter.
Future<String?> getDirectoryPath({
String? initialDirectory,
String? confirmButtonText,
}) {
throw UnimplementedError('getDirectoryPath() has not been implemented.');
}

/// Opens a file dialog for loading directories and returns multiple directory paths.
/// Opens a file dialog for loading directories and returns multiple directory
/// paths.
///
/// Returns an empty list if the user cancels the operation.
// TODO(stuartmorgan): Switch to FileDialogOptions if we ever need to
// duplicate this to add a parameter.
Future<List<String>> getDirectoryPaths({
String? initialDirectory,
String? confirmButtonText,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:flutter/foundation.dart' show immutable;

/// Configuration options for any file selector dialog.
@immutable
class FileDialogOptions {
/// Creates a new options set with the given settings.
const FileDialogOptions({this.initialDirectory, this.confirmButtonText});

/// The initial directory the dialog should open with.
final String? initialDirectory;

/// The label for the button that confirms selection.
final String? confirmButtonText;
}

/// Configuration options for a save dialog.
@immutable
class SaveDialogOptions extends FileDialogOptions {
/// Creates a new options set with the given settings.
const SaveDialogOptions(
{super.initialDirectory, super.confirmButtonText, this.suggestedName});

/// The suggested name of the file to save or open.
final String? suggestedName;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:flutter/foundation.dart' show immutable;

import 'x_type_group.dart';

export 'x_type_group.dart';

/// The response from a save dialog.
@immutable
class FileSaveLocation {
/// Creates a result with the given [path] and optional other dialog state.
const FileSaveLocation(this.path, {this.activeFilter});

/// The path to save to.
final String path;

/// The currently active filter group, if any.
///
/// This is null on platforms that do not support user-selectable filter
/// groups in save dialogs (for example, macOS), or when no filter groups
/// were provided when showing the dialog.
final XTypeGroup? activeFilter;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@
// found in the LICENSE file.

export 'package:cross_file/cross_file.dart';
export 'x_type_group/x_type_group.dart';
export 'file_dialog_options.dart';
export 'file_save_location.dart';
export 'x_type_group.dart';
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ repository: https://github.com/flutter/packages/tree/main/packages/file_selector
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+file_selector%22
# NOTE: We strongly prefer non-breaking changes, even at the expense of a
# less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes
version: 2.5.1
version: 2.6.0

environment:
sdk: ">=2.18.0 <4.0.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ void main() {
// Store the initial instance before any tests change it.
final FileSelectorPlatform initialInstance = FileSelectorPlatform.instance;

group('$FileSelectorPlatform', () {
group('FileSelectorPlatform', () {
test('$MethodChannelFileSelector() is the default instance', () {
expect(initialInstance, isInstanceOf<MethodChannelFileSelector>());
});
Expand All @@ -20,7 +20,7 @@ void main() {
});
});

group('#GetDirectoryPaths', () {
group('getDirectoryPaths', () {
test('Should throw unimplemented exception', () async {
final FileSelectorPlatform fileSelector = ExtendsFileSelectorPlatform();

Expand All @@ -29,6 +29,30 @@ void main() {
}, throwsA(isA<UnimplementedError>()));
});
});

test('getSaveLocation falls back to getSavePath by default', () async {
final FileSelectorPlatform fileSelector =
OldFileSelectorPlatformImplementation();

final FileSaveLocation? result = await fileSelector.getSaveLocation();

expect(result?.path, OldFileSelectorPlatformImplementation.savePath);
expect(result?.activeFilter, null);
});
}

class ExtendsFileSelectorPlatform extends FileSelectorPlatform {}

class OldFileSelectorPlatformImplementation extends FileSelectorPlatform {
static const String savePath = '/a/path';
// Only implement the deprecated getSavePath.
@override
Future<String?> getSavePath({
List<XTypeGroup>? acceptedTypeGroups,
String? initialDirectory,
String? suggestedName,
String? confirmButtonText,
}) async {
return savePath;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ class SaveTextPage extends StatelessWidget {

Future<void> _saveFile() async {
final String fileName = _nameController.text;
// TODO(stuartmorgan): Update this to getSaveLocation in the next federated
// change PR.
// ignore: deprecated_member_use
final String? path = await FileSelectorPlatform.instance.getSavePath(
// Operation was canceled by the user.
suggestedName: fileName,
Expand Down