feat: add direction prop to PaperProvider and useLocale hook for RTL support#4921
feat: add direction prop to PaperProvider and useLocale hook for RTL support#4921hristototov wants to merge 3 commits intov6from
Conversation
…into @hristototov/feat/improve_RTL_handling
|
The mobile version of example app from this branch is ready! You can see it here. |
| <PortalHost> | ||
| <SettingsProvider value={settingsValue}> | ||
| <ThemeProvider theme={theme}>{children}</ThemeProvider> | ||
| <LocaleProvider value={{ direction }}> |
There was a problem hiding this comment.
We get a new object on every render and this affects FPS and re-render counts on screens that use Paper components.
We already memoizes theme and settingsValue. Do the same for this too.
|
Hey @hristototov, thank you for your pull request 🤗. The documentation from this branch can be viewed here. |
satya164
left a comment
There was a problem hiding this comment.
Thanks for the PR. I left some comments. In addition, lets do the following:
- Export a
LocaleProviderlike done forThemeProvider, so user can wrap a specific subtree in a different locale if needed. The defaults etc., can live in this component directly instead of agetDefaultDirectionexport - Inherit the nearest locale in
Portal.tsxas done for the theme - Update documentation. Here is an example https://reactnavigation.org/docs/navigation-container/#direction
| export const LocaleContext = React.createContext<LocaleContextValue>({ | ||
| direction: getDefaultDirection(), | ||
| }); | ||
|
|
||
| export const { Provider: LocaleProvider } = LocaleContext; | ||
|
|
||
| export const useLocale = () => React.useContext(LocaleContext); |
There was a problem hiding this comment.
It shouldn't provide a default locale in the context itself. It must be provided when rendering the provider so it stays consistent, and components outside the tree properly throw.
The useLocale hook should throw an error when used outside the provider.
| export const LocaleContext = React.createContext<LocaleContextValue>({ | |
| direction: getDefaultDirection(), | |
| }); | |
| export const { Provider: LocaleProvider } = LocaleContext; | |
| export const useLocale = () => React.useContext(LocaleContext); | |
| export const LocaleContext = React.createContext<LocaleContextValue | undefined>(); | |
| export const { Provider: LocaleProvider } = LocaleContext; | |
| export function useLocale() { | |
| const locale = React.useContext(LocaleContext); | |
| if (locale === undefined) { | |
| throw new Error("Couldn't find a locale. Is your component inside PaperProvider?"); | |
| } | |
| return locale; | |
| } |
Motivation
Components in react-native-paper read RTL state directly from
I18nManager.getConstants().isRTL. This has two issues:I18nManageris a no-op on React Native Web, so RTL breaks entirely on WebSolution
New:
useLocale()hookDefaults to I18nManager.getConstants().isRTL when not provided, so existing apps are unaffected. Setting the prop does not call I18nManager.forceRTL or modify any native state; it only tells Paper which direction to use for rendering.
Related issue
N/A
Notion link
Test plan
Remove the prop and verify the app falls back to I18nManager (default behaviour unchanged)