Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Fix demos’ RTL ↔ LTR toggling in Safari #2348

Merged
merged 2 commits into from
Mar 6, 2018
Merged

Fix demos’ RTL ↔ LTR toggling in Safari #2348

merged 2 commits into from
Mar 6, 2018

Conversation

swashcap
Copy link
Contributor

@swashcap swashcap commented Mar 6, 2018

  • Problem: The right-to-left ↔ left-to-right toggling on the top-app-bar demo is broken in Safari 11.0.3 (on macOS 10.13.3)

  • Solution: Don’t remove the dir attribute on body in the demo: change its value to ltr when the “RTL” checkbox is unchecked

  • Testing:

    1. Get the fix branch set up: 
      git clone [email protected]:swashcap/material-components-web.git mcw2
cd mcw2
npm install
npm run dev
    2. Open http://localhost:8080/top-app-bar.html in Safari
    3. Confirm the “RTL” checkbox works after checked and unchecked

@swashcap
fix(demos): correct RTL ↔ LTR top app bar toggling in Safari
5e1d96a 
This addresses a bug in Safari 11 where removing the `dir=rtl` attribute
on the `body` element does not revert the direction change on many child
elements. Explicitly settint the `dir` back to `ltr` corrects this
behavior.
@swashcap
Copy link
Contributor Author

swashcap commented Mar 6, 2018

Just to elaborate, here’s the current behavior I see:

current-behavior

@swashcap
Copy link
Contributor Author

swashcap commented Mar 6, 2018

Confirmed behavior on iOS 11.2.6. Here’s a reproduction via the iPhone SE simulator:

ios-sim-whoa

@swashcap
Copy link
Contributor Author

swashcap commented Mar 6, 2018 •
edited
Loading

Confirmed this still exists in Safari Tech Preview 50 (for macOS 10.13 build). Wonder if there’s a ticket for a browser bug…

@codecov-io
Copy link

codecov-io commented Mar 6, 2018 •
edited
Loading

Codecov Report

Merging #2348 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2348   +/-   ##
=======================================
  Coverage   98.85%   98.85%           
=======================================
  Files         100      100           
  Lines        4118     4118           
  Branches      527      527           
=======================================
  Hits         4071     4071           
  Misses         47       47

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab85736...32d6283. Read the comment docs.

@swashcap
Copy link
Contributor Author

swashcap commented Mar 6, 2018

Ran through all the demos with RTL checkboxes and found that the card demo also has the s bug. 32d6283 addresses the issue.

Before:

before

After:

after

@swashcap swashcap changed the title fix(demos): correct RTL ↔ LTR top app bar toggling in Safari Fix demos’ RTL ↔ LTR toggling in Safari Mar 6, 2018
@kfranqueiro kfranqueiro self-requested a review March 6, 2018 15:17
@kfranqueiro kfranqueiro self-assigned this Mar 6, 2018
@kfranqueiro
Copy link
Contributor

Thanks so much for the detailed explanation and checking through the rest of the demo pages to find other instances of the problem! This one's definitely a trip; I was able to reproduce it in isolation, too: https://codepen.io/kfranqueiro/pen/mXZNew

@kfranqueiro kfranqueiro merged commit b9000a4 into material-components:master Mar 6, 2018
@swashcap swashcap deleted the bug/top-app-bar-rtl-safari branch March 6, 2018 17:41
acdvorak added a commit that referenced this pull request Mar 8, 2018
@acdvorak
WIP Squashed commit of the following:
5ccfdc1 
commit f17e0d3
Author: Will Ernest <[email protected]>
Date:   Thu Mar 8 13:20:48 2018 -0500

    fix(text-field): Clicking label should focus textfield (#2353)

commit 77b15f4
Author: Bonnie Zhou <[email protected]>
Date:   Wed Mar 7 16:29:57 2018 -0800

    refactor(button): Remove compact variant (#2361)

    BREAKING CHANGE: The compact variant of MDC Button is removed.

commit 35a5cfc
Author: Will Ernest <[email protected]>
Date:   Tue Mar 6 19:54:23 2018 -0500

    fix(toolbar): Fix colors for svg icons. Update custom-toolbar demo (#2331)

    SVG icons in the toolbar will now use the same color as the font icons.

commit dc52201
Author: Andrew C. Dvorak <[email protected]>
Date:   Tue Mar 6 16:00:12 2018 -0800

    fix(theme): Move @alternate annotations for Closure Stylesheets (#2355)

    `@alternate` annotations need to come before the _second_ property. Otherwise, Closure Compiler strips the first property (it does not output it at all).

commit 3c04419
Author: aprigogin <[email protected]>
Date:   Tue Mar 6 14:29:12 2018 -0800

    fix(button): icon in rtl should have margin right flipped. (#2346)

commit b9000a4
Author: Cory Reed <[email protected]>
Date:   Tue Mar 6 07:37:40 2018 -0800

    fix(demos): Correct RTL/LTR toggling in demos in Safari (#2348)

commit ab85736
Author: Andrew C. Dvorak <[email protected]>
Date:   Mon Mar 5 19:00:11 2018 -0500

    fix: Use `var` instead of `const` in menu demo (#2345)

    Safari 9.x and IE 10 do not support `const`

commit dc3d69f
Author: aprigogin <[email protected]>
Date:   Mon Mar 5 15:22:31 2018 -0800

    fix(rtl): Adding noflip annotations to fix downstream rtl issues (#2344)

    * fix(rtl): Adding noflip annotations to fix downstream rtl issues

    * fix(rtl): remove changes to button, will be in separate PR

    * fix(rtl): remove changes to button, will be in separate PR - second attempt.

    * fix(rtl): removed extra whitespace

commit eb4138e
Author: Patrick RoDee <[email protected]>
Date:   Mon Mar 5 14:10:46 2018 -0800

    docs: Update CHANGELOG.md

commit f478610
Author: Patrick RoDee <[email protected]>
Date:   Mon Mar 5 14:10:30 2018 -0800

    chore: Publish

commit 78408bb
Author: Andrew C. Dvorak <[email protected]>
Date:   Mon Mar 5 16:13:02 2018 -0500

    fix: Use `var` instead of `const` in demos/ready.js (#2343)

    Safari 9.x and IE 10 do not support `const`, and `ready.js` is not transpiled to ES5.

commit 49a9449
Author: Will Ernest <[email protected]>
Date:   Mon Mar 5 12:21:54 2018 -0500

    chore(select): Fix JS example in Readme (#2332)

commit bc17291
Author: Will Ernest <[email protected]>
Date:   Fri Mar 2 13:21:07 2018 -0500

    feat(top app bar): Add short top app bar always collapsed feature (#2327)

commit 0ba7d10
Author: Matty Goo <[email protected]>
Date:   Fri Mar 2 11:33:19 2018 -0500

    fix(text-field): disable validation check in setRequired (#2201)

    BREAKING CHANGE: removed setRequired and isRequired from foundation.

commit c14d244
Author: Will Ernest <[email protected]>
Date:   Fri Mar 2 10:37:18 2018 -0500

    chore(select): Fix ID values in demo (#2330)

    Remove unused ID and changed duplicated ID values. Also fixed the parentheses in RTL.

commit ecf4060
Author: Bonnie Zhou <[email protected]>
Date:   Thu Mar 1 16:38:41 2018 -0500

    feat(chips): Change chip color when selected (#2329)

    BREAKING CHANGE: The `mdc-chip--activated` class, `mdc-chip-activated-ink-color` Sass mixin, and the `toggleActive` methods on `MDCChip`/`MDCChipSet` have been renamed to `mdc-chip--selected`, `mdc-chip-selected-ink-color`, and `toggleSelected`, respectively.

commit 4b24b51
Author: Matty Goo <[email protected]>
Date:   Wed Feb 28 15:20:19 2018 -0500

    chore(floating-label): separate label module from text-field (#2237)

    BREAKING CHANGE: must use `.mdc-floating-label` selector instead of `.mdc-text-field__label`

commit fd8d8d9
Author: Will Ernest <[email protected]>
Date:   Wed Feb 28 14:21:55 2018 -0500

    feat(top-app-bar): Implement short top app bar (#2290)

    Adds the short top app bar variant to the top app bar.

commit a9f9bf2
Author: Kenneth G. Franqueiro <[email protected]>
Date:   Wed Feb 28 13:06:41 2018 -0500

    docs(select): Fix front matter for label sub-component (#2323)
@dependabot-preview dependabot-preview bot mentioned this pull request Mar 19, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kfranqueiro kfranqueiro kfranqueiro approved these changes

Assignees

@kfranqueiro kfranqueiro

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@swashcap @codecov-io @kfranqueiro