[flutter_adaptive_scaffold] Support RTL #4204

ahmednfwela · 2023-06-13T11:47:43Z

This PR removes the hardcoded rtl directionality placed on top of AdaptiveScaffold for an unknown reason.

RTL	LTR

Changes in this PR

Core: Removed this hardcoded Directionality widget

packages/packages/flutter_adaptive_scaffold/lib/src/adaptive_scaffold.dart

Lines 491 to 494 in 0507297

    
           return Directionality( 
        
             textDirection: TextDirection.ltr, 
        
             child: Scaffold( 
        
               appBar: widget.drawerBreakpoint.isActive(context) && widget.useDrawer

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

Question for reviewers: should the next version be 0.1.5 or 0.2.0 ?

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

gspencergoog

Thanks for fixing that! We definitely shouldn't be hard coding a directionality.

ahmednfwela · 2023-06-13T16:53:41Z

@gspencergoog what should the next version be?

gspencergoog · 2023-06-13T16:57:45Z

what should the next version be?

I'd say 0.1.5

justinmc

LGTM 👍

@gspencergoog It looks like there were some formatting changes in this PR, just making sure you are ok with that.

packages/flutter_adaptive_scaffold/example/lib/main.dart

gspencergoog · 2023-06-14T18:33:10Z

It looks like there were some formatting changes in this PR, just making sure you are ok with that.

Yes, I'm OK with that. The packages repo enforces automatic formatting with dart format, so I don't worry about formatting in this repo: if the checks pass, then the formatting is correct. The indentation changes in the example markdown are fine too, since that just removes unnecessary whitespace.

justinmc · 2023-06-14T21:17:10Z

Got it, thanks for the info!

ahmednfwela · 2023-06-15T02:04:18Z

@gspencergoog since the new version relies on dart 3/flutter 3.10, should this have been a minor version upgrade instead of a patch ?

flutter/packages@f9314a3...59d93d6 2023-06-16 stuartmorgan@google.com [tool] Add command aliases (flutter/packages#4207) 2023-06-16 stuartmorgan@google.com [google_map_flutter] Fix map object regression due to async changes (flutter/packages#4171) 2023-06-16 goderbauer@google.com [url_launcher] Add ignores for deprecated member to test (flutter/packages#4220) 2023-06-15 engine-flutter-autoroll@skia.org Roll Flutter from 95be76a to b0188cd (10 revisions) (flutter/packages#4221) 2023-06-15 34871572+gmackall@users.noreply.github.com [camera_android] Upgrading roboelectric from 4.5 to 4.10.3 (flutter/packages#4018) 2023-06-15 47866232+chunhtai@users.noreply.github.com [go_router] Fixes bug that GoRouterState in top level redirect doesn'â�¦ (flutter/packages#4173) 2023-06-15 47866232+chunhtai@users.noreply.github.com [go_router]Updates documentations around GoRouter.of, GoRouter.maybeOf, and BuildContext extension. (flutter/packages#4176) 2023-06-15 stuartmorgan@google.com [tool] Support code excerpts for any .md file (flutter/packages#4212) 2023-06-15 baptiste.dupuch@gmail.com [webview_flutter] Add support for limitsNavigationsToAppBoundDomains (flutter/packages#4026) 2023-06-15 yk3372@gmail.com [webview_flutter][webview_flutter_android] Add android support for handling geolocation permissions (flutter/packages#3795) 2023-06-15 tarrinneal@gmail.com [image_picker] add getMedia method (flutter/packages#3892) 2023-06-15 tarrinneal@gmail.com [image_picker] getMedia platform implementations (flutter/packages#4175) 2023-06-14 31859944+LongCatIsLooong@users.noreply.github.com Ignore `textScaleFactor` deprecation (flutter/packages#4209) 2023-06-14 stuartmorgan@google.com [pigeon] Enable Obj-C integration tests in CI (flutter/packages#4215) 2023-06-14 63286031+ahmednfwela@users.noreply.github.com [flutter_adaptive_scaffold] Support RTL (flutter/packages#4204) 2023-06-14 engine-flutter-autoroll@skia.org Manual roll Flutter from 09b7e56 to 95be76a (14 revisions) (flutter/packages#4214) 2023-06-14 engine-flutter-autoroll@skia.org Roll Flutter (stable) from 682aa38 to 796c8ef (5 revisions) (flutter/packages#4213) 2023-06-14 engine-flutter-autoroll@skia.org Roll Flutter from 353b8bc to 09b7e56 (17 revisions) (flutter/packages#4206) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

gspencergoog · 2023-07-05T16:55:43Z

@ahmednfwela No, it's not adding functionality, it's a bug fix. If we consider the updated dependency a breaking change, then it would be a major version update, but since this is still a 0. release, we won't bump it to 1.0 yet.

ahmednfwela added 6 commits June 13, 2023 14:30

Remove hardcoded TextDirection

e9e8678

Add tests to ensure RTL direction is respected

6a17efb

unskip fixed tests

fa7a448

Make Row and Column consts

1df5896

Added language switch to the main example to demonstrate RTL support.

640d046

update changelog

b322486

ahmednfwela requested a review from gspencergoog as a code owner June 13, 2023 11:47

github-actions bot added the p: flutter_adaptive_scaffold label Jun 13, 2023

ahmednfwela mentioned this pull request Jun 13, 2023

No RTL support in flutter_adaptive_scaffold package flutter/flutter#119661

Closed

ahmednfwela added 3 commits June 13, 2023 15:48

partial CI fix

91b1cf5

partial CI fix

4a0b873

add missing #enddocregion Example

b8a2617

gspencergoog approved these changes Jun 13, 2023

View reviewed changes

format

e3a21fc

justinmc approved these changes Jun 14, 2023

View reviewed changes

packages/flutter_adaptive_scaffold/example/lib/main.dart Outdated Show resolved Hide resolved

ahmednfwela and others added 2 commits June 14, 2023 20:30

nit

387cc1d

Merge branch 'main' into adaptive_scaffold_rtl

e8d0a1d

gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 14, 2023

auto-submit bot merged commit b7f1ad4 into flutter:main Jun 14, 2023

ahmednfwela deleted the adaptive_scaffold_rtl branch June 15, 2023 00:30

engine-flutter-autoroll mentioned this pull request Jun 16, 2023

Roll Packages from f9314a3b833a to 59d93d64cb83 (18 revisions) flutter/flutter#129014

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 16, 2023

b7f1ad4 [flutter_adaptive_scaffold] Support RTL (flutter/packages#4204)

2c0726f

engine-flutter-autoroll mentioned this pull request Jun 16, 2023

Roll Packages from f9314a3b833a to 59d93d64cb83 (18 revisions) flutter/flutter#129021

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 16, 2023

b7f1ad4 [flutter_adaptive_scaffold] Support RTL (flutter/packages#4204)

cbaaaf5

This was referenced Jul 7, 2023

[Tool] New tool to download android dependencies #4408

Merged

[camera] tree fix format 2023 05 12 #4450

Closed

[tooling] mark infra steps as infra steps in yaml files #4473

Merged

reidbaker mentioned this pull request Jul 26, 2023

[Tooling] Add google owned cache for dependencies as an option in ci #4567

Merged

11 tasks

reidbaker mentioned this pull request Aug 4, 2023

[All] Expand artifact hub to all plugins #4645

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flutter_adaptive_scaffold] Support RTL #4204

[flutter_adaptive_scaffold] Support RTL #4204

Uh oh!

ahmednfwela commented Jun 13, 2023 •

edited

Loading

Uh oh!

gspencergoog left a comment

Uh oh!

ahmednfwela commented Jun 13, 2023

Uh oh!

gspencergoog commented Jun 13, 2023 •

edited

Loading

Uh oh!

justinmc left a comment

Uh oh!

Uh oh!

gspencergoog commented Jun 14, 2023

Uh oh!

justinmc commented Jun 14, 2023

Uh oh!

ahmednfwela commented Jun 15, 2023

Uh oh!

gspencergoog commented Jul 5, 2023 •

edited

Loading

Uh oh!

Uh oh!

	return Directionality(
	textDirection: TextDirection.ltr,
	child: Scaffold(
	appBar: widget.drawerBreakpoint.isActive(context) && widget.useDrawer

[flutter_adaptive_scaffold] Support RTL #4204

[flutter_adaptive_scaffold] Support RTL #4204

Uh oh!

Conversation

ahmednfwela commented Jun 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes in this PR

Related issues

Pre-launch Checklist

Uh oh!

gspencergoog left a comment

Choose a reason for hiding this comment

Uh oh!

ahmednfwela commented Jun 13, 2023

Uh oh!

gspencergoog commented Jun 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gspencergoog commented Jun 14, 2023

Uh oh!

justinmc commented Jun 14, 2023

Uh oh!

ahmednfwela commented Jun 15, 2023

Uh oh!

gspencergoog commented Jul 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ahmednfwela commented Jun 13, 2023 •

edited

Loading

gspencergoog commented Jun 13, 2023 •

edited

Loading

gspencergoog commented Jul 5, 2023 •

edited

Loading