-
Notifications
You must be signed in to change notification settings - Fork 128
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
new option dropdown below menu as described in #278 #279
base: master
Are you sure you want to change the base?
Conversation
…forces the dropdown menu to only be displayed beneath the dropdown button, even if it means to make the dropdown menu scrollable
…ocumentation in README.md
…ssible size if needed
I fixed the bug that broke the expected behaviour. |
I appreciate your effort anyway :) Can you share the |
Here is a simplified version of the example shown above, without the loading algorithms that currently depend on a own server: import 'package:dropdown_button2/dropdown_button2.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
//import 'package:flutterfontchooser/flutterfontchooser.dart';
void main() {
runApp(const MyApp());
}
class MyApp extends StatelessWidget {
const MyApp({super.key});
// This widget is the root of your application.
@override
Widget build(BuildContext context) {
return MaterialApp(
title: 'Test',
theme: ThemeData(
colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
useMaterial3: true,
),
home: const MyHomePage(),
);
}
}
class MyHomePage extends StatelessWidget {
const MyHomePage({super.key});
@override
Widget build(BuildContext context) {
return const Scaffold(
body: Center(
child: DropdownButtonHideUnderline(
child: Column(
mainAxisAlignment: MainAxisAlignment.end,
children: [
Text("Width: ..."),
Text("Height: 500px"),
Text("..."),
Row(
mainAxisAlignment: MainAxisAlignment.spaceEvenly,
children: [
Text("Bold"),
Text("Italic"),
],
),
FontChooserDropdownMenuWithoutServerAccessAndNoFonts(),
//FontChooserDropdownMenu(),
SizedBox(height: 400),
],
),
),
),
);
}
}
//a dropdown widget that allows the user to choose a font, and displays the font name in the dropdown.
//the fonts get dynamically loaded from the server
class FontChooserDropdownMenuWithoutServerAccessAndNoFonts extends StatefulWidget {
//const FontChooserDropdownMenu({super.key, super.controller});
const FontChooserDropdownMenuWithoutServerAccessAndNoFonts({super.key});
@override
State<FontChooserDropdownMenuWithoutServerAccessAndNoFonts> createState() =>
_FontChooserDropdownMenuWithoutServerAccessAndNoFontsState();
}
class _FontChooserDropdownMenuWithoutServerAccessAndNoFontsState
extends State<FontChooserDropdownMenuWithoutServerAccessAndNoFonts> {
//final Map<SearchResponseEntryComparable, DropdownItem<SearchResponseEntryComparable>>
final Map<String, DropdownItem<String>>
_items = {};
void _loadItems() {
/*Set<SearchResponseEntryComparable> oldItems = _items.keys.toSet();
bool changed = false;
for (var value in controller.allFontsIfLoaded) {
SearchResponseEntryComparable comparable =
SearchResponseEntryComparable(value);
if (!oldItems.contains(comparable)) {
changed = true;
_items[comparable] = DropdownItem<SearchResponseEntryComparable>(
value: comparable,
child: DropdownRowWidget(
key: Key("${value.family}-dropdown-row-widget"),
selectedFont: selectedFont,
thisFont: value,
),
);
}
oldItems.remove(comparable);
}
for (var item in oldItems) {
_items.remove(item);
}
if (changed && mounted) {
setState(() {});
}*/
for(int i=0; i<1500;i++){
_items["Font $i"] = DropdownItem<String>(
value: "Font $i",
child: DropdownRowWidget(
key: Key("Font $i-dropdown-row-widget"),
selectedFont: selectedFont,
thisFont: "Font $i",
),
);
}
setState(() {});
}
@override
void initState() {
super.initState();
ServicesBinding.instance.keyboard.addHandler(_onKey);
Future.microtask(_loadItems);
}
/*@override
void onController() {
var s = searchResponseEntryComparableFromSearchResponseEntry(
controller.selectedFont?.searchResponseEntry);
if (s != selectedFont.value) {
selectedFont.value = s;
}
_loadItems();
}*/
@override
void dispose() {
ServicesBinding.instance.keyboard.removeHandler(_onKey);
super.dispose();
}
//final ValueNotifier<SearchResponseEntryComparable?> selectedFont =
final ValueNotifier<String?> selectedFont =
ValueNotifier(null);
TextEditingController searchController = TextEditingController();
FocusNode searchFocusNode = FocusNode();
bool _onKey(KeyEvent event) {
if (_menuOpen && !searchFocusNode.hasFocus && event is KeyDownEvent) {
String? key = event.character;
if (key != null &&
event.logicalKey != LogicalKeyboardKey.enter &&
event.logicalKey != LogicalKeyboardKey.space &&
event.logicalKey != LogicalKeyboardKey.tab) {
if (event.logicalKey == LogicalKeyboardKey.backspace) {
if (searchController.text.isNotEmpty) {
searchController.text = "";
searchFocusNode.requestFocus();
return true;
}
searchFocusNode.requestFocus();
return false;
}
searchController.text += key;
searchFocusNode.requestFocus();
return true;
}
}
return false;
}
bool _menuOpen = false;
@override
Widget build(BuildContext context) {
//return DropdownButton2<SearchResponseEntryComparable>(
return DropdownButton2<String>(
valueListenable: selectedFont,
barrierCoversButton: false,
dropdownOnlyBelowButton: true,
onMenuStateChange: (open) {
_menuOpen = open;
},
buttonStyleData: const ButtonStyleData(
width: 260,
height: 50,
),
dropdownSearchData: DropdownSearchData(
searchController: searchController,
searchBarWidgetHeight: 50,
searchBarWidget: Container(
height: 50,
padding: const EdgeInsets.only(
top: 8,
bottom: 4,
right: 8,
left: 8,
),
child: TextFormField(
maxLines: 1,
controller: searchController,
focusNode: searchFocusNode,
decoration: InputDecoration(
isDense: true,
contentPadding: const EdgeInsets.symmetric(
horizontal: 10,
vertical: 8,
),
hintText: 'Search for a font...',
hintStyle: const TextStyle(fontSize: 12),
border: OutlineInputBorder(
borderRadius: BorderRadius.circular(8),
),
),
),
),
noResultsWidget: const Padding(
padding: EdgeInsets.all(8),
child: Text('No Font Found!'),
),
searchMatchFn:
//(DropdownItem<SearchResponseEntryComparable> entry, String search) {
(DropdownItem<String> entry, String search) {
//return entry.value?.family
return entry.value?.toLowerCase()
.contains(search.toLowerCase()) ??
false;
},
),
iconStyleData: const IconStyleData(
icon: Icon(
Icons.arrow_drop_down,
color: Colors.black45,
),
),
dropdownStyleData: DropdownStyleData(
decoration: BoxDecoration(
borderRadius: BorderRadius.circular(15),
),
),
menuItemStyleData: const MenuItemStyleData(
padding: EdgeInsets.symmetric(horizontal: 16),
),
hint: const Text(
'Select a font',
style: TextStyle(fontSize: 14),
),
/*onChanged: (SearchResponseEntryComparable? newValue) {
selectedFont.value = newValue;
if (newValue == null) {
controller.selectedFont = null;
return;
}
Future.microtask(() async {
controller.selectedFont =
await LoadedFont.fromSearchResponseEntry(newValue.entry);
});
},*/
onChanged: (String? newValue) {
selectedFont.value = newValue;
},
items: _items.values.toList(),
);
}
}
class DropdownRowWidget extends StatefulWidget {
//final SearchResponseEntry thisFont;
final String thisFont;
//final ValueNotifier<SearchResponseEntryComparable?> selectedFont;
final ValueNotifier<String?> selectedFont;
final double width;
const DropdownRowWidget(
{super.key,
required this.thisFont,
this.width = 200,
required this.selectedFont});
@override
State<DropdownRowWidget> createState() => _DropdownRowWidgetState();
}
class _DropdownRowWidgetState extends State<DropdownRowWidget> {
bool _isFontLoaded = false;
bool _isLoading = false;
@override
Widget build(BuildContext context) {
return Row(
mainAxisAlignment: MainAxisAlignment.spaceBetween,
children: [
SizedBox(
width: widget.width,
child: Text(widget.thisFont,
/*style: Theme.of(context)
.textTheme
.apply(fontFamily: widget.thisFont.menuFontFamily)
.bodyMedium*/),
),
if (!_isFontLoaded && !_isLoading)
const Icon(
Icons.cloud_download_outlined,
color: Colors.grey,
size: 24,
),
/*if (!_isFontLoaded && _isLoading)
LoadingAnimationWidget.dotsTriangle(
color: Colors.grey,
size: 24,
),*/
],
);
}
void _check() {
if(!mounted){
return;
}
bool s = false;
//bool i = isFontLoaded(widget.thisFont);
bool i =false;
if (i != _isFontLoaded) {
_isFontLoaded = i;
s = true;
}
//i = isLoadingFont(widget.thisFont);
i= false;
if (i != _isLoading) {
_isLoading = i;
s = true;
}
if (s) {
setState(() {});
}
}
@override
void initState() {
super.initState();
_check();
//FontLoaderNotifier.instance.addListener(_check);
widget.selectedFont.addListener(_check);
}
@override
void didUpdateWidget(DropdownRowWidget oldWidget) {
super.didUpdateWidget(oldWidget);
_check();
}
@override
void dispose() {
//FontLoaderNotifier.instance.removeListener(_check);
super.dispose();
}
}
|
I tried achieving the behaviour of my option via the max-height property, but for example opening the virtual keyboard on mobile devices or resizing the window on desktop changes the available max height, but setting the max height only gets applied each time the menu is reopened. (or is there a way to apply it directly without reopening the menu?) |
The specified |
A thought to improve UX with the new proposed option enabled: We could check if the space forced with the option enabled is enough to display the menu (with maybe 1 to 2 rows visible at least) and if not fall back to the default behaviour (option disabled / non existent) |
On the other side, I get it that this PR would complicate testing, and overall constantly performing a good and understandable experience for those who don't want to read the source code of every lib they use. What do you think of introducing a new callback function to calculate the available height? This way, this package could allow this special use case (and potentionally others), while taking away the responsibility for a good UX in this case from the package. The callback function could be defined as: double? Function(double, EdgeInsets,Rect)? calculateMenuAvailableHeight; and could be used in the 'getMenuAvailableHeight' function in dropdown_route.dart like: double getMenuAvailableHeight(
double availableHeight,
EdgeInsets mediaQueryPadding,
Rect buttonRect,
) {
double? callbackHeight = calculateMenuAvailableHeight != null
? calculateMenuAvailableHeight(
availableHeight, mediaQueryPadding, buttonRect)
: null;
return math.max(
0.0,
callbackHeight ??
(availableHeight - mediaQueryPadding.vertical - _kMenuItemHeight),
);
} |
It's desired that the dropdown can go above the button when there isn't enough space underneath and I think that's the correct behavior. Forcing the menu to always open beneath the button will lead to issues that's hard to catch and bad UX. i.e: Screen.Recording.2024-06-01.at.7.04.14.PM.movScreen.Recording.2024-06-01.at.7.07.05.PM.movScreen.Recording.2024-06-01.at.7.09.33.PM.mov |
I agree that the dropdown should go above the button when there isnt't enough space underneath, the problem I have is that the current implementation always decides that there isn't enough space, even if there is enough space to display the menu with multiple rows and the search widget underneath and going above the button only gains 1-2 rows. Issues like in this video
can also happen if the developer sets a very low maximum height - following your line of argumentation we should add checks to make sure the maximum height is big enough to display the dropdown menu and ignore/increase this maximum height passed by the app developer if it is to low for an 'good' UX - or we should remove it for simplicit to enforce an 'good' UX. IMO there is no reason to do this - after all this project is just a tool used by many apps, and the responsibility for a good UX ultimately lies with the app developer. Of course this project should encourage good UX and support the app developer to implement it, but giving the app developer more freedom to change the look of this package in a way that also enables bad UX doesn't necessarily mean bad UX in the app, no it also enables better UX for certain use cases. I get it that my proposed parameter 'dropdownOnlyBelowButton' may indeed allow bad UX if not understood correctly by the app developer, but this doesn't mean the app developer should not have this option in a way that decreases the likelihood of a bad UX.
If the app developer takes the risk and implements his own 'calculateMenuAvailableHeight' I think this package is not responsible for the negative effects caused by the app developers' implementation - just like this package is not responsible for an inapropiate max height that leads to bad UX. I would be happy to create a new PR for this, just dont't want to waste that time. |
The current implementation decides if there isn't enough space as expected. If you set max height to I think this pr is not about opening below button but more about "dynamically resizing the menu" which I'm skeptical about it for the mentioned issues and for the fact that I don't find a real use case for it, i.e:
No, this is a different thing. This will be a decision by the user that won't work fine at first place. The proposed option would work fine, but i.e if the user minimize the window or when scrolling the button to the bottom, it'll lead to issues like overflow or the user will press the button and menu won't show at all. |
imo, a better proposal that will avoid the mentioned issues for the developer/user experience, is a I'll need some time to think more about it :) |
See #278