[flutter_adaptive_scaffold] Support RTL (flutter#4204)

This PR removes the hardcoded rtl directionality placed on top of `AdaptiveScaffold` for an unknown reason. | RTL | LTR | |-----|-----| | ![image](https://github.com/flutter/packages/assets/63286031/10adac59-3f9a-4f4d-85da-939efbb47ae8) | ![image](https://github.com/flutter/packages/assets/63286031/c8c1af75-4787-473c-a397-a64d2f9b1e88) | ## Changes in this PR - Core: Removed this hardcoded `Directionality` widget https://github.com/flutter/packages/blob/050729760b251e315af01876cc7d7de5dcfba0e9/packages/flutter_adaptive_scaffold/lib/src/adaptive_scaffold.dart#L491-L494 - Core: Added tests to ensure text direction is passed correctly - Core: Modified `example/adaptive_scaffold_demo.dart` to demonstrate support for RTL - Side: There were some tests that were getting skipped, I have enabled them since their related issues landed on stable. - Side: Stopped ignoring `prefer_const_constructors` since it landed in stable as well. I made the commit messages as descriptive as possible so that it's easier to review relevant changes. ## Related issues - Supersedes flutter#3602 - Fixes flutter#119661 Question for reviewers: should the next version be 0.1.5 or 0.2.0 ?
engine-flutter-autoroll · Jun 14, 2023 · b7f1ad4 · b7f1ad4
1 parent c34ca5d
commit b7f1ad4
Show file tree

Hide file tree

Showing 8 changed files with 515 additions and 463 deletions.
diff --git a/packages/flutter_adaptive_scaffold/CHANGELOG.md b/packages/flutter_adaptive_scaffold/CHANGELOG.md
@@ -1,5 +1,6 @@
-## NEXT
+## 0.1.5
 
+* Added support for Right-to-left (RTL) directionality.
 * Fixes stale ignore: prefer_const_constructors.
 * Updates minimum supported SDK version to Flutter 3.10/Dart 3.0.
 

diff --git a/packages/flutter_adaptive_scaffold/README.md b/packages/flutter_adaptive_scaffold/README.md
@@ -137,110 +137,108 @@ displayed and the entrance animation and exit animation.
 
 <?code-excerpt "adaptive_layout_demo.dart (Example)"?>
 ```dart
-    // AdaptiveLayout has a number of slots that take SlotLayouts and these
-    // SlotLayouts' configs take maps of Breakpoints to SlotLayoutConfigs.
-    return AdaptiveLayout(
-      // Primary navigation config has nothing from 0 to 600 dp screen width,
-      // then an unextended NavigationRail with no labels and just icons then an
-      // extended NavigationRail with both icons and labels.
-      primaryNavigation: SlotLayout(
-        config: <Breakpoint, SlotLayoutConfig>{
-          Breakpoints.medium: SlotLayout.from(
-            inAnimation: AdaptiveScaffold.leftOutIn,
-            key: const Key('Primary Navigation Medium'),
-            builder: (_) => AdaptiveScaffold.standardNavigationRail(
-              selectedIndex: selectedNavigation,
-              onDestinationSelected: (int newIndex) {
-                setState(() {
-                  selectedNavigation = newIndex;
-                });
-              },
-              leading: const Icon(Icons.menu),
-              destinations: destinations
-                  .map((_) => AdaptiveScaffold.toRailDestination(_))
-                  .toList(),
-              backgroundColor: navRailTheme.backgroundColor,
-              selectedIconTheme: navRailTheme.selectedIconTheme,
-              unselectedIconTheme: navRailTheme.unselectedIconTheme,
-              selectedLabelTextStyle: navRailTheme.selectedLabelTextStyle,
-              unSelectedLabelTextStyle: navRailTheme.unselectedLabelTextStyle,
-            ),
-          ),
-          Breakpoints.large: SlotLayout.from(
-            key: const Key('Primary Navigation Large'),
-            inAnimation: AdaptiveScaffold.leftOutIn,
-            builder: (_) => AdaptiveScaffold.standardNavigationRail(
-              selectedIndex: selectedNavigation,
-              onDestinationSelected: (int newIndex) {
-                setState(() {
-                  selectedNavigation = newIndex;
-                });
-              },
-              extended: true,
-              leading: Row(
-                mainAxisAlignment: MainAxisAlignment.spaceAround,
-                children: const <Widget>[
-                  Text(
-                    'REPLY',
-                    style: TextStyle(color: Color.fromARGB(255, 255, 201, 197)),
-                  ),
-                  Icon(Icons.menu_open)
-                ],
-              ),
-              destinations: destinations
-                  .map((_) => AdaptiveScaffold.toRailDestination(_))
-                  .toList(),
-              trailing: trailingNavRail,
-              backgroundColor: navRailTheme.backgroundColor,
-              selectedIconTheme: navRailTheme.selectedIconTheme,
-              unselectedIconTheme: navRailTheme.unselectedIconTheme,
-              selectedLabelTextStyle: navRailTheme.selectedLabelTextStyle,
-              unSelectedLabelTextStyle: navRailTheme.unselectedLabelTextStyle,
-            ),
-          ),
-        },
+// AdaptiveLayout has a number of slots that take SlotLayouts and these
+// SlotLayouts' configs take maps of Breakpoints to SlotLayoutConfigs.
+return AdaptiveLayout(
+  // Primary navigation config has nothing from 0 to 600 dp screen width,
+  // then an unextended NavigationRail with no labels and just icons then an
+  // extended NavigationRail with both icons and labels.
+  primaryNavigation: SlotLayout(
+    config: <Breakpoint, SlotLayoutConfig>{
+      Breakpoints.medium: SlotLayout.from(
+        inAnimation: AdaptiveScaffold.leftOutIn,
+        key: const Key('Primary Navigation Medium'),
+        builder: (_) => AdaptiveScaffold.standardNavigationRail(
+          selectedIndex: selectedNavigation,
+          onDestinationSelected: (int newIndex) {
+            setState(() {
+              selectedNavigation = newIndex;
+            });
+          },
+          leading: const Icon(Icons.menu),
+          destinations: destinations
+              .map((_) => AdaptiveScaffold.toRailDestination(_))
+              .toList(),
+          backgroundColor: navRailTheme.backgroundColor,
+          selectedIconTheme: navRailTheme.selectedIconTheme,
+          unselectedIconTheme: navRailTheme.unselectedIconTheme,
+          selectedLabelTextStyle: navRailTheme.selectedLabelTextStyle,
+          unSelectedLabelTextStyle: navRailTheme.unselectedLabelTextStyle,
+        ),
       ),
-      // Body switches between a ListView and a GridView from small to medium
-      // breakpoints and onwards.
-      body: SlotLayout(
-        config: <Breakpoint, SlotLayoutConfig>{
-          Breakpoints.small: SlotLayout.from(
-            key: const Key('Body Small'),
-            builder: (_) => ListView.builder(
-              itemCount: children.length,
-              itemBuilder: (BuildContext context, int index) => children[index],
-            ),
+      Breakpoints.large: SlotLayout.from(
+        key: const Key('Primary Navigation Large'),
+        inAnimation: AdaptiveScaffold.leftOutIn,
+        builder: (_) => AdaptiveScaffold.standardNavigationRail(
+          selectedIndex: selectedNavigation,
+          onDestinationSelected: (int newIndex) {
+            setState(() {
+              selectedNavigation = newIndex;
+            });
+          },
+          extended: true,
+          leading: const Row(
+            mainAxisAlignment: MainAxisAlignment.spaceAround,
+            children: <Widget>[
+              Text(
+                'REPLY',
+                style: TextStyle(color: Color.fromARGB(255, 255, 201, 197)),
+              ),
+              Icon(Icons.menu_open)
+            ],
           ),
-          Breakpoints.mediumAndUp: SlotLayout.from(
-            key: const Key('Body Medium'),
-            builder: (_) =>
-                GridView.count(crossAxisCount: 2, children: children),
-          )
-        },
+          destinations: destinations
+              .map((_) => AdaptiveScaffold.toRailDestination(_))
+              .toList(),
+          trailing: trailingNavRail,
+          backgroundColor: navRailTheme.backgroundColor,
+          selectedIconTheme: navRailTheme.selectedIconTheme,
+          unselectedIconTheme: navRailTheme.unselectedIconTheme,
+          selectedLabelTextStyle: navRailTheme.selectedLabelTextStyle,
+          unSelectedLabelTextStyle: navRailTheme.unselectedLabelTextStyle,
+        ),
       ),
-      // BottomNavigation is only active in small views defined as under 600 dp
-      // width.
-      bottomNavigation: SlotLayout(
-        config: <Breakpoint, SlotLayoutConfig>{
-          Breakpoints.small: SlotLayout.from(
-            key: const Key('Bottom Navigation Small'),
-            inAnimation: AdaptiveScaffold.bottomToTop,
-            outAnimation: AdaptiveScaffold.topToBottom,
-            builder: (_) => AdaptiveScaffold.standardBottomNavigationBar(
-              destinations: destinations,
-              currentIndex: selectedNavigation,
-              onDestinationSelected: (int newIndex) {
-                setState(() {
-                  selectedNavigation = newIndex;
-                });
-              },
-            ),
-          )
-        },
+    },
+  ),
+  // Body switches between a ListView and a GridView from small to medium
+  // breakpoints and onwards.
+  body: SlotLayout(
+    config: <Breakpoint, SlotLayoutConfig>{
+      Breakpoints.small: SlotLayout.from(
+        key: const Key('Body Small'),
+        builder: (_) => ListView.builder(
+          itemCount: children.length,
+          itemBuilder: (BuildContext context, int index) => children[index],
+        ),
       ),
-    );
-  }
-}
+      Breakpoints.mediumAndUp: SlotLayout.from(
+        key: const Key('Body Medium'),
+        builder: (_) =>
+            GridView.count(crossAxisCount: 2, children: children),
+      )
+    },
+  ),
+  // BottomNavigation is only active in small views defined as under 600 dp
+  // width.
+  bottomNavigation: SlotLayout(
+    config: <Breakpoint, SlotLayoutConfig>{
+      Breakpoints.small: SlotLayout.from(
+        key: const Key('Bottom Navigation Small'),
+        inAnimation: AdaptiveScaffold.bottomToTop,
+        outAnimation: AdaptiveScaffold.topToBottom,
+        builder: (_) => AdaptiveScaffold.standardBottomNavigationBar(
+          destinations: destinations,
+          currentIndex: selectedNavigation,
+          onDestinationSelected: (int newIndex) {
+            setState(() {
+              selectedNavigation = newIndex;
+            });
+          },
+        ),
+      )
+    },
+  ),
+);
 ```
 
 Both of the examples shown here produce the same output:

diff --git a/packages/flutter_adaptive_scaffold/example/lib/adaptive_layout_demo.dart b/packages/flutter_adaptive_scaffold/example/lib/adaptive_layout_demo.dart
@@ -2,9 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// TODO(goderbauer): Remove this ignore when this package requires Flutter 3.8 or later.
-// ignore_for_file: prefer_const_constructors
-
 import 'package:flutter/material.dart';
 import 'package:flutter_adaptive_scaffold/flutter_adaptive_scaffold.dart';
 
@@ -58,8 +55,8 @@ class _MyHomePageState extends State<MyHomePage> {
       children: <Widget>[
         const Divider(color: Colors.black),
         const SizedBox(height: 10),
-        Row(
-          children: const <Widget>[
+        const Row(
+          children: <Widget>[
             SizedBox(width: 27),
             Text('Folders', style: TextStyle(fontSize: 16)),
           ],
@@ -74,8 +71,8 @@ class _MyHomePageState extends State<MyHomePage> {
               iconSize: 21,
             ),
             const SizedBox(width: 21),
-            Flexible(
-              child: const Text(
+            const Flexible(
+              child: Text(
                 'Freelance',
                 overflow: TextOverflow.ellipsis,
               ),
@@ -92,8 +89,8 @@ class _MyHomePageState extends State<MyHomePage> {
               iconSize: 21,
             ),
             const SizedBox(width: 21),
-            Flexible(
-              child: const Text(
+            const Flexible(
+              child: Text(
                 'Mortgage',
                 overflow: TextOverflow.ellipsis,
               ),
@@ -198,9 +195,9 @@ class _MyHomePageState extends State<MyHomePage> {
                 });
               },
               extended: true,
-              leading: Row(
+              leading: const Row(
                 mainAxisAlignment: MainAxisAlignment.spaceAround,
-                children: const <Widget>[
+                children: <Widget>[
                   Text(
                     'REPLY',
                     style: TextStyle(color: Color.fromARGB(255, 255, 201, 197)),
@@ -260,6 +257,6 @@ class _MyHomePageState extends State<MyHomePage> {
         },
       ),
     );
-    // #enddocregion
+    // #enddocregion Example
   }
 }