Conversation
|
Looking good! For the hovering/selection, the shape should also be rounded, as specified in the navigation drawer specs. |
|
@yaymalaga Thanks, updated inkwell shape |
darrenaustin
left a comment
There was a problem hiding this comment.
Looks good. Nice work!
I have a bunch of minor comments and a couple of suggestions about the API below. Thx!
packages/flutter/lib/src/material/navigation_destination_data.dart
Outdated
Show resolved
Hide resolved
packages/flutter/lib/src/material/navigation_destination_data.dart
Outdated
Show resolved
Hide resolved
packages/flutter/lib/src/material/navigation_destination_data.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Why is this being made public? Do we need it someplace else?
If we need to have it as a generally available widget, we should move it into its own file and have tests for it.
There was a problem hiding this comment.
I changed it back to private and add another private one in NavigationDrawer.
There was a problem hiding this comment.
This could be written in a more functional way with something like:
int destinationIndex = 0;
Widget _wrapChild(Widget child) {
if (child is NavigationDrawerDestination) {
return SelectableAnimatedBuilder(
duration: const Duration(milliseconds: 500),
isSelected: destinationIndex == selectedIndex,
builder: (BuildContext context, Animation<double> animation) {
return NavigationDestinationInfo(
index: destinationIndex,
// ...
destinationIndex += 1;
}
return child;
}
final List<Widget> wrappedChildren = children.map(_wrapChild).toList();There was a problem hiding this comment.
Actually after testing it I found out that adding " destinationIndex += 1" in "_wrapChild" will make it not working. Maybe because the function is run lazily?
So I will keep the "for" loop to make it work.
examples/api/lib/material/navigation_drawer/navigation_drawer.0.dart
Outdated
Show resolved
Hide resolved
examples/api/lib/material/navigation_drawer/navigation_drawer.0.dart
Outdated
Show resolved
Hide resolved
examples/api/lib/material/navigation_drawer/navigation_drawer.0.dart
Outdated
Show resolved
Hide resolved
examples/api/lib/material/navigation_drawer/navigation_drawer.0.dart
Outdated
Show resolved
Hide resolved
examples/api/lib/material/navigation_drawer/navigation_drawer.0.dart
Outdated
Show resolved
Hide resolved
|
Here's a version of the example that eliminates the need for the NavigationDestinationData class. I don't think that the cost of adding NavigationDestinationData balances the benefit. Hopefully we do not need public NavigationDestinationInfo or SelectableAnimatedBuilder classes either. class ExampleDestination {
const ExampleDestination(this.label, this.icon, this.selectedIcon);
final String label;
final IconData icon;
final IconData selectedIcon;
}
const List<ExampleDestination> destinations = <ExampleDestination>[
ExampleDestination('page 0', Icon(Icons.widgets_outlined), Icon(Icons.widgets)),
ExampleDestination('page 1', Icon(Icons.format_paint_outlined), Icon(Icons.format_paint)),
ExampleDestination('page 2', Icon(Icons.invert_text_snippet_outlined), Icon(Icons.text_snippet)),
ExampleDestination('page 3', Icon(Icons.invert_colors_on_outlined), Icon(Icons.opacity)),
];
void main() {
runApp(
MaterialApp(
title: 'NavigationDrawer Example',
debugShowCheckedModeBanner: false,
theme: ThemeData(useMaterial3: true),
home: const NavigationDrawerExample(),
),
);
}
class NavigationDrawerExample extends StatefulWidget {
const NavigationDrawerExample({super.key});
@override
State<NavigationDrawerExample> createState() => _NavigationDrawerExampleState();
}
class _NavigationDrawerExampleState extends State<NavigationDrawerExample> {
final GlobalKey<ScaffoldState> scaffoldKey = GlobalKey<ScaffoldState>();
int screenIndex = 0;
late bool showNavigationDrawer;
void handleScreenChanged(int selectedScreen) {
setState(() {
screenIndex = selectedScreen;
});
}
void openDrawer() {
scaffoldKey.currentState!.openEndDrawer();
}
Widget buildBottomBarScaffold(){
return Scaffold(
body: Center(
child: Column(
mainAxisAlignment: MainAxisAlignment.spaceEvenly,
children: <Widget>[
Text('Page Index = $screenIndex'),
],
),
),
bottomNavigationBar: NavigationBar(
selectedIndex: screenIndex,
onDestinationSelected: (int index) {
setState(() {
screenIndex = index;
});
},
destinations: destinations
.map((ExampleDestination destination) {
return NavigationBarDestination(
label: destination.label,
icon: destination.icon,
selectedIcon: destination.selectedIcon,
tooltip: data.label,
);
})
.toList(),
),
);
}
Widget buildDrawerScaffold(BuildContext context){
return Scaffold(
key: scaffoldKey,
body: SafeArea(
bottom: false,
top: false,
child: Row(
children: <Widget>[
Padding(
padding: const EdgeInsets.symmetric(horizontal: 5),
child: NavigationRail(
minWidth: 50,
destinations: destinations
.map((ExampleDestination destination) {
return NavigationRailDestination(
label: Text(destination.label),
icon: destination.icon,
selectedIcon: destination.selectedIcon,
);
})
.toList(),
selectedIndex: screenIndex,
useIndicator: true,
onDestinationSelected: (int index) {
setState(() {
screenIndex = index;
});
},
),
),
const VerticalDivider(thickness: 1, width: 1),
Expanded(
child: Column(
mainAxisAlignment: MainAxisAlignment.spaceEvenly,
children: <Widget>[
Text('Page Index = $screenIndex'),
ElevatedButton(
onPressed: openDrawer,
child: const Text('Open Drawer'),
),
],
),
),
],
),
),
endDrawer: NavigationDrawer(
onDestinationSelected: handleScreenChanged,
selectedIndex: screenIndex,
children: <Widget>[
Padding(
padding: const EdgeInsets.fromLTRB(28, 16, 16, 10),
child: Text(
'Header',
style: Theme.of(context).textTheme.titleSmall,
),
),
destinations
.map((ExampleDestination destination) {
return NavigationDrawerDestination(
label: destination.label,
icon: destination.icon,
selectedIcon: destination.selectedIcon,
);
}),
const Padding(
padding: EdgeInsets.fromLTRB(28, 16, 28, 10),
child: Divider(),
),
],
),
);
}
@override
void didChangeDependencies() {
super.didChangeDependencies();
showNavigationDrawer = MediaQuery.of(context).size.width >= 450;
}
@override
Widget build(BuildContext context) {
return showNavigationDrawer ? buildDrawerScaffold(context) : buildBottomBarScaffold();
}
} |
|
The hover/selection highlights for NavigationDrawerItems in the video below the description are rectangular, but they should be stadium shaped, same as the items themselves. |
|
@HansMuller Thanks, I updated the example and updated the video in description |
59ca958 to
a579c6b
Compare
drawer add token Update navigation_drawer.dart Update navigation_drawer.dart Update navigation_rail.dart navigation_destination_Data add example update drawer lint update example update animation update comments update comments add tests update comments lint lint lint inkwell shape lint lint fix some tests Update packages/flutter/lib/src/material/navigation_drawer.dart Update packages/flutter/lib/src/material/navigation_drawer.dart Update packages/flutter/lib/src/material/navigation_drawer.dart Update packages/flutter/lib/src/material/navigation_drawer.dart Update packages/flutter/lib/src/material/navigation_drawer.dart Update packages/flutter/lib/src/material/navigation_drawer.dart Update packages/flutter/lib/src/material/navigation_drawer.dart apply fix Update navigation_drawer.0.dart apply fix shadowColor Update packages/flutter/lib/src/material/navigation_drawer.dart Update packages/flutter/lib/src/material/navigation_drawer.dart Update packages/flutter/lib/src/material/navigation_drawer.dart Update packages/flutter/lib/src/material/navigation_drawer.dart Update packages/flutter/lib/src/material/navigation_drawer.dart Update navigation_drawer.dart update example comment remove NavigationDestinationData Add label widget update label to widget lint lint Add drawerControllerScope update template small fix lint update test lint lint Update drawer_theme.dart lint Update navigation_drawer.dart Update theme_data_test.dart Co-Authored-By: Darren Austin <[email protected]> Co-Authored-By: Hans Muller <[email protected]>
a579c6b to
682e808
Compare
darrenaustin
left a comment
There was a problem hiding this comment.
Looks good. Getting close. I just have a few suggestions and questions below.
Co-authored-by: Darren Austin <[email protected]>
Co-authored-by: Darren Austin <[email protected]>
Co-authored-by: Darren Austin <[email protected]>
Co-authored-by: Darren Austin <[email protected]>
Co-authored-by: Darren Austin <[email protected]>
|
I created a new PR from the patch of this one to bypass the CLA check: #115668 |

issue: #103551
Pre-launch Checklist
///).drawer-recording-2.mov
If you need help, consider asking for advice on the #hackers-new channel on Discord.