Skip to content
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

Make TOC text direction always match article text direction. #1397

Merged
merged 5 commits into from May 3, 2017

Conversation

montehurd
Copy link
Contributor

@montehurd montehurd commented May 2, 2017

https://phabricator.wikimedia.org/T149066

When device lang is LTR...

Before

The TOC was still LTR when the article was RTL:
before mov

After

The TOC is RTL when the article is RTL:
after mov

@montehurd montehurd force-pushed the regression/rtl-toc/T149066 branch from 6e4af08 to e6bfffb Compare May 2, 2017 21:01
@montehurd montehurd changed the title WIP Fix TOC when device lang is LTR and article lang is RTL. May 2, 2017
@montehurd montehurd removed the WIP label May 2, 2017
@montehurd montehurd changed the title Fix TOC when device lang is LTR and article lang is RTL. Fix TOC text direction when device lang is LTR and article lang is RTL. May 2, 2017
@montehurd montehurd added the WIP label May 2, 2017
@montehurd
Copy link
Contributor Author

Oops forgot to test when device lang is RTL and you load a LTR article...

…ches.

This commit fixes both mismatch possibilies:
- device RTL with LTR article
- device LTR with RTL article
@montehurd montehurd removed the WIP label May 2, 2017
@montehurd montehurd changed the title Fix TOC text direction when device lang is LTR and article lang is RTL. Make TOC text direction always match article text direction. May 2, 2017
@@ -32,6 +32,8 @@ open class WMFTableOfContentsViewController: UIViewController,
WMFTableOfContentsAnimatorDelegate {

let tableOfContentsFunnel: ToCInteractionFunnel

let semanticContentAttribute: UISemanticContentAttribute
Copy link
Contributor Author

@montehurd montehurd May 2, 2017

Choose a reason for hiding this comment

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

Considered naming this semanticContentAttributeOverride... any naming preference?

Copy link
Contributor

Choose a reason for hiding this comment

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

actually I like semanticContentAttributeOverride

@joewalsh
Copy link
Contributor

joewalsh commented May 3, 2017

retest this please

@joewalsh joewalsh merged commit 86f1560 into develop May 3, 2017
@joewalsh joewalsh deleted the regression/rtl-toc/T149066 branch May 3, 2017 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants