Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Android/iOS] support locale LayoutDirection on MasterDetailPage #3992

Closed
wants to merge 6 commits into from
Closed

[Android/iOS] support locale LayoutDirection on MasterDetailPage #3992

wants to merge 6 commits into from

Conversation

paymicro
Copy link
Contributor

@paymicro paymicro commented Oct 4, 2018

Description of Change

Support locale LayoutDirection on MasterDetailPage.

Issues Resolved

API Changes

None

Platforms Affected

  • All

Behavioral/Visual Changes

Controls of MasterDetalPage shown on the correct side.

Before/After Screenshots

Screencast:
http://recordit.co/nlOXpzGDrU

Android
Before
before

After
screenshot_3

iOS

Before
screenshot_9

After
screenshot_15

screenshot_14

Testing Procedure

Android

  • setup RTL locale on device
  • run Control gallery
  • check hamburger position on MasterDetailPage

iOS

  • setup RTL locale on device
  • run Control gallery
  • tap/click on "Master" title
  • check red box position

PR Checklist

  • Has automated tests
  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard

@rmarinho
Copy link
Member

rmarinho commented Oct 4, 2018

build

@rmarinho
Copy link
Member

rmarinho commented Oct 4, 2018

Can we add iOS screenshots too?

@samhouts samhouts removed has-stacktrace Bugs without reproductions but that contain a stack trace. labels Oct 4, 2018
@samhouts samhouts changed the title [Android] support locale LayoutDirection on MasterDetailPage [Android/iOS] support locale LayoutDirection on MasterDetailPage Oct 4, 2018
Copy link
Member

@StephaneDelcroix StephaneDelcroix left a comment

Choose a reason for hiding this comment

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

I'm usure about this. do we follow device's RTL setting usually, or do we only follow our own FlowLayout property ?

@paymicro
Copy link
Contributor Author

paymicro commented Oct 5, 2018

do we follow device's RTL setting usually, or do we only follow our own FlowLayout property ?

Apparently we only follow our own FlowLayout property and ignore device RTL...
But root element isn't setted to device RTL if it is implicit

@xamarin-release-manager xamarin-release-manager added the API-change Heads-up to reviewers that this PR may contain an API change label Oct 15, 2018
@PureWeen
Copy link
Contributor

@paymicro can you rebase to fix conflict please

@StephaneDelcroix StephaneDelcroix removed their assignment Oct 26, 2018
@samhouts samhouts self-assigned this Oct 26, 2018
@samhouts
Copy link
Member

We need to make sure that we keep this statement true:

As with UWP, respecting the device's layout direction based on the selected language/locale is the developer's explicit choice and does not happen automatically. Set the FlowDirection of the root View/Page to the Device.FlowDirection value. All other child views with MatchParent will inherit this value. (#1222)

@samhouts samhouts added the e/4 🕓 4 label Nov 2, 2018
@PureWeen PureWeen assigned PureWeen and unassigned samhouts Nov 2, 2018
@PureWeen PureWeen self-requested a review November 2, 2018 17:48
@rmarinho
Copy link
Member

@paymicro can you confirm this only happens if setting the property explicitly like @samhouts said?

@paymicro
Copy link
Contributor Author

@rmarinho yes. Set FlowDirection works correctly. http://recordit.co/pcNTb0wQnP
This PR is fixes if FlowDirection has not been set or it has been set to MatchParent and it has no parent, then it will be taken from device's layout.

@rmarinho
Copy link
Member

"This PR is fixes if FlowDirection has not been set or it has been set"

But I think we should not settle locale if the user didn't specified

@samhouts
Copy link
Member

This is not the correct fix. We don't want to use the device locale automatically. We want the user to do that explictly if that is what they want to do. We need to figure out why the hamburger button doesn't respect the FlowDirection property instead of trying to set the FlowDirection property based on the device locale. Thanks!

@samhouts samhouts closed this Nov 21, 2018
@paymicro paymicro deleted the fix-gh2818-rtl-master-hamburger branch November 22, 2018 08:23
@samhouts samhouts added this to the 3.5.0 milestone Jan 23, 2019
@candidodmv
Copy link

@samhouts I need only the navigation menu of MasterDetail page stay on rightToLeft side, everthing else stay on leftToRight, how can I succed after this update? because when I was targeting to version 3.4 working very well.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Android] Right-to-Left MasterDetail in Xamarin.Forms v3 Hamburger icon issue
7 participants