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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

@material-ui/styles RTL support #14283

Closed
2 tasks done
mbrevda opened this issue Jan 23, 2019 · 18 comments
Closed
2 tasks done

@material-ui/styles RTL support #14283

mbrevda opened this issue Jan 23, 2019 · 18 comments
Labels
docs Improvements or additions to the documentation
Milestone

Comments

@mbrevda
Copy link

mbrevda commented Jan 23, 2019

Does the new ThemeProvider have full RTL support?

  • This is not a v0.x issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 馃

With RTL enabled TextFileds, including labels and placeholders, should be right aligned.

Current Behavior 馃槸

TextFileds seem left aligned
screen shot 2019-01-23 at 10 17 41 am

Steps to Reproduce 馃暪

https://codesandbox.io/s/wq787oxnml

Your Environment 馃寧

Tech Version
Material-UI v3.9.0
Material-UI styles 3.0.0-alpha.8
React 16.8.0-alpha.1
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 23, 2019

@mbrevda Yes, it does:

import React from "react";
import ReactDOM from "react-dom";
import { createMuiTheme } from "@material-ui/core/styles";
import { StylesProvider, ThemeProvider, jssPreset } from "@material-ui/styles";
import { create } from "jss";
import rtl from "jss-rtl";
import Demo from "./demo";

const jss = create({ plugins: [...jssPreset().plugins, rtl()] });
const theme = createMuiTheme({
  direction: "rtl"
});

ReactDOM.render(
  <StylesProvider jss={jss}>
    <ThemeProvider theme={theme}>
      <Demo />
    </ThemeProvider>
  </StylesProvider>,
  document.querySelector("#root")
);

https://codesandbox.io/s/material-demo-3uhoj

capture d ecran 2019-01-23 a 12 01 58

We will have to upgrade the RTL guide documentation with v4.

@oliviertassinari oliviertassinari changed the title ThemeProvider RTL support @material-ui/styles RTL support Jan 23, 2019
@oliviertassinari oliviertassinari added this to the v4 milestone Jan 23, 2019
@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Jan 23, 2019
@mbrevda
Copy link
Author

mbrevda commented Jan 23, 2019

Brilliant, thanks so much!

@eps1lon
Copy link
Member

eps1lon commented Jan 29, 2019

As I understand the commonet of @mbrevda this issue can be closed.

@eps1lon eps1lon closed this as completed Jan 29, 2019
@mbrevda
Copy link
Author

mbrevda commented Jan 29, 2019

I thought this was staying open as a to-do for updating the docs?

Otherwise, my issue is resolved, thanks.

@oliviertassinari
Copy link
Member

Yes, we will need to update the documentation.

@eps1lon
Copy link
Member

eps1lon commented Jan 29, 2019

I think https://material-ui.com/css-in-js/advanced/#jss-plugins is already covering this.

@oliviertassinari
Copy link
Member

@eps1lon Yes, for @material-ui/core/styles, but not for @material-ui/styes.

@eps1lon
Copy link
Member

eps1lon commented Jan 29, 2019

The linked section is about @material-ui/styles. It is also discoverable by simply searching for rtl.

I think we can add a link to that section to https://material-ui.com/customization/css-in-js/#jss to improve discoverability.

@oliviertassinari
Copy link
Member

@eps1lon
Copy link
Member

eps1lon commented Jan 29, 2019

So we have one for core/styles and just styles. Looks good to me.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 29, 2019

From my perspective, we only have a single true RTL documentation page, it's the guide, that is using the legacy styles module. The other references are incomplet, following them won't be enough.

@eps1lon
Copy link
Member

eps1lon commented Jan 29, 2019

I don't understand why not. Since our components are listening to the a custom theme then obviously people need to change the direction in that theme too. How to enable rtl with the styles package is already documented. How to provide the mui theme is documented. The full guide is documented and the parts that use core/styles are also documented in styles. Do you want to duplicate every documentation that is using core/styles with just styles?

@oliviertassinari
Copy link
Member

Do you want to duplicate every documentation that is using core/styles with just styles?

No, I don't. It was a reminder to update the guide documentation once we release v4. Also, people often have a hard time to put all the pieces together. We often have to give them something working outside of the box.

However, we might not need it after all. We have another problem to solve, how do we handle the default theme? @material-ui/styles doesn't have a default theme. We could keep @material-ui/core/styles to encapsulate it.

@TrueMoein
Copy link

@mbrevda Yes, it does:

import "./bootstrap.js";
import React from "react";
import ReactDOM from "react-dom";
import { createMuiTheme } from "@material-ui/core/styles";
import { StylesProvider, ThemeProvider } from "@material-ui/styles";
import { create } from "jss";
import rtl from "jss-rtl";
import { jssPreset } from "@material-ui/core/styles";
import Demo from "./demo";

const jss = create({ plugins: [...jssPreset().plugins, rtl()] });
const theme = createMuiTheme({
  typography: { useNextVariants: true },
  direction: "rtl"
});

ReactDOM.render(
  <StylesProvider jss={jss}>
    <ThemeProvider theme={theme}>
      <Demo />
    </ThemeProvider>
  </StylesProvider>,
  document.querySelector("#root")
);

https://codesandbox.io/s/kwowy9278v

capture d ecran 2019-01-23 a 12 01 58

We will have to upgrade the RTL guide documentation with v4.

Hi @oliviertassinari, can you test this example again? It seems not working now.
Thanks.

@oliviertassinari
Copy link
Member

@TrueMoein I have updated my original comment to reflect how it should be done with v4, latest.

@aminesi
Copy link

aminesi commented Jul 23, 2019

Hi @oliviertassinari did you test your solution with typescript cause i get this error:

Argument of type '{ plugins: (Plugin | { onProcessStyle(style: any, rule: any, sheet: any): any; })[]; }' is not assignable to parameter of type 'Partial'.
Types of property 'plugins' are incompatible.
Type '(Plugin | { onProcessStyle(style: any, rule: any, sheet: any): any; })[]' is not assignable to type 'readonly JSSPlugin[]'.
Type 'Plugin | { onProcessStyle(style: any, rule: any, sheet: any): any; }' is not assignable to type 'JSSPlugin'.
Type 'Plugin' is not assignable to type 'JSSPlugin'.
Index signature is missing in type 'Plugin'

when i try to declare jss const. thanks

@Iamthewaseem
Copy link

You saved my life!!!!

@sadeghesfahani
Copy link

if you don't want to make the whole project right to left and you want to be able to change the direction dynamically, I have made a component to wrap your elements with it and make them direction sensitive, you can find it here:
https://github.com/sadeghesfahani/direction-sensitive-material-ui

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

No branches or pull requests

7 participants