jaredh159 / tailwind-react-native-classnames

simple, expressive API for tailwindcss + react-native
2.01k stars 82 forks source link

Dark Mode #15

Closed mraghuram closed 2 years ago

mraghuram commented 3 years ago

Hi, can we enable dark mode ?

jaredh159 commented 3 years ago

Not currently supported, but I'm open to adding support or accepting a PR.

I don't use dark mode in any of the RN apps I maintain, so I'm not totally sure what exactly would be involved. How would you imagine the API would work? In RN we don't have the concept of an operating system media query, not exactly at least. How would we signal to the module that we're in dark mode?

Do you have an app that you want to support both dark and light mode for? Is it an explicit user pref in your app? Or does your app read the preference from some global/os setting? Can you comment on how it's handled in iOS and Android?

mraghuram commented 3 years ago

Jared, thanks for such a prompt response.

In the current app I am building, there is a request to see if we can switch themes. For this I am exploring libraries such as UIKitten and React Native elements. I am a big fan of tailwind and hence was exploring that alongside as well.

So in a nutshell, still at an exploration stage. On a side note, do you have recommendations of tailwind vs the UI kits I mentioned?

jaredh159 commented 3 years ago

Gotcha. I have no experience with UIKitten or React Native elements, so I can't speak to that. But I can say that using Tailwind in RN is not an all-or-nothing commitment. I use it in all the places where it's convenient, and where tailwind utilities exist (and work in RN), and then I just drop down to passing style objects whenever I need to. I find that about 90% of the time I'm able to stay in tailwind. FWIW.

mraghuram commented 3 years ago

That helps. Thanks. And btw, amazing work on the library, thanks for your effort.

BTW, is there a way we can get VScode intellisense with tw?

jaredh159 commented 3 years ago

BTW, is there a way we can get VScode intellisense with tw?

No intellisense currently, but I'm sure it could be built into a vscode plugin or something, if someone had the inclination. Or maybe there is a way this package could be made to play nicely with the main tailwind vscode plugin, I honestly haven't looked into it.

Currently though, if you are dev-ing a RN app, and you type in an incorrect (unknown) tailwind classname, you'll immediately get a warning in your simulator or device, so you get pretty fast feedback if you type a utility incorrectly, or try to use on that doesn't exist.

mraghuram commented 3 years ago

BTW, have you looked at useColorScheme option? https://reactnative.dev/docs/0.63/usecolorscheme

This hook may provide you the details about the current user setting (Appearance)

jaredh159 commented 3 years ago

Yeah, that hook looks promising. I'm still a bit unsure exactly how this would be implemented. Reacting to a change in the user color scheme and ensuring that this package becomes aware, and re-renders take place, etc., sounds a bit complex. And since it's not a use-case I've needed yet for my apps, I'm going to wait until someone is either willing to tackle it in a PR, or at least give me a detailed and well thought out implementation strategy that can get me going in the right direction.

I'll leave this issue open for now.

crysfel commented 3 years ago

This library looks awesome! I love the way you added platform specific styles. I'm also interested on adding dark mode support, ideally it should work as tailwind does, with styles like:

bg-blue-500 dark:bg-blue-100

I really like the approach you've taken with this library, will give it a try on the dark mode feature.

Regards

jaredh159 commented 3 years ago

I'm toying around with adding support for this in a version 2.0 of the library I'm starting to work on. I think I see a way to dynamically respond to color scheme changes using the useColorScheme() hook, but I'm struggling with how the implementation would work, because of differences between the web and react native. I'm hoping a few of the folks watching this issue could think carefully about this with me and offer any suggestions.

The core problem is that in normal web tailwind, you can do a set of classes like bg-white dark:bg-black, and the way tailwind is written, the dark: variant has a higher CSS specificity, so it will trump the non-dark variant when the media query for prefers-color-scheme matches.

That means, in normal web tailwind, you just apply both classes and the browser's CSS engine takes care of understanding what you mean based on CSS specificity. There's an implicit merging of the two class names handled by the CSS engine.

It's not so simple in RN land, there is no CSS specificity we can fall back on here. Right now the way the library works is by building up a single RN style object using object spreading, like so:

const style = {};
style = { ...subStyleParsedFromClassName };
style = { ...subStyleParsedFromAnotherClassName };
// [...etc]

I could just rely on the order of the object merging -- if the color scheme did not match, nothing would get merged in, but if it did match, it would get merged in later, thus overriding values set from the non-dark variant. The problem with this approach is that it relies on the developer supplying the classes in a specific order, which doesn't feel like a great option. I could try to sort the classes before applying them, I suppose, but that sounds possibly dicy, and might have a non-negligable runtime cost, especially because I'm trying to do some aggressive internal caching in v2 for performance reasons.

The other option seems like we could introduce a complementary light: modifier, and thus force the developer to be explicit about which styles are opting into color scheme awareness. I would assume that light: would apply in all situations except when the color scheme is known to be dark (i.e., when the scheme is light, or not specified). The advantage of this approach is that it's very simple and explicit, no magic behind the scenes. But it does present a departure from the mental model of the web version of tailwind UI.

So I guess I'm seeing 3 options:

Option 1: Use the web-style api of bg-white dark:bg-black and document that the developer is responsible for ensuring the dark:* classes come after other classes that should be overriden.

Option 2: Use the web-style api of bg-white dark:bg-black and try to handle the ordering issue internally within the library, hopefully in a deterministic way that doesn't produce any other order-related issues.

Option 3: Use a more explicit prefixing of light:bg-white dark:bg-black thus forcing the developer to explicitly declare what properties should be affected by the color scheme.

Or...

Option 4: Something I haven't thought up yet... πŸ€”

Definitely would love some input on this issue.

jittuu commented 3 years ago

Could you please add poll for the options? May be with reaction. πŸ™‚

I vote for Option 3.

ilyavf commented 3 years ago

Voting for Option 1 - easy concept and no extra "magic".

Option 2 is OK too if the ordering is simple (e.g. just sorts dark to the end).

Option 3 is more verbose and less appealing because we will have 3 different cases to think about - general, light and dark.

ilyavf commented 3 years ago

Actually, useColorScheme is not enough if dark/light mode needs to be switched on the application level.

So far, I found that @react-navigation/native's useTheme hook allows more flexibility. The idea is that useColorScheme is only used to choose the default theme, and then theme is passed to NavigationContainer and later can be switched programmatically by simply using react state. See reactnavigation docs using-the-operating-system-preferences and using-the-current-theme for more details.

ilyavf commented 3 years ago

Oh, useTheme is just using React.useContext behind the scene, makes sense.

jamesallain commented 3 years ago

Option 1 for me. RN developers are aware that order matters with StyleSheet.

iangregsondev commented 3 years ago

Hi,

I am currently using tailwind for web but would love to use it here on RN .. On the web I am using a vs code plugin to automatically change the order of the class names.. https://marketplace.visualstudio.com/items?itemName=heybourn.headwind

Not sure if I could still use it - probably not.

But anyway, I think it would be great if there was an option other than the one that requires devs to put classes in a certain order,

So I think Option 2.

That way we don't need to think about copying things in a certain order.

Just my 2 cents :-)

Thanks.

ngasull commented 3 years ago

Seems reasonable to follow tailwind's own rule on this topic :

whenever dark mode is enabled on the user’s operating system, dark:{class} classes will take precedence over unprefixed classes.

This rule looks like option 2 you suggested. I don't think we should be afraid of precedence oddities because the rule is simple : dark:{class} takes over {class}.

Option 1 looks totally out of question to me because CSS class order never had any effect of the rendering. I think that us developers intuitively expect this behavior in RN as well while using CSS classes from tailwind.

I love the work done on this library and I am so happy to be able to benefit from tailwind in react native, so: thank you so much :heart:

@jaredh159 I would undertake the PR with pleasure if your time is too limited to work on it. I would be interested to see v2 code as well!

jaredh159 commented 3 years ago

@ngasull thanks for the offer with the PR. I've got dark-mode implemented in my private V2 branch already, and I did go with option 2. I'll get an alpha ready within a few days, and then you're welcome to look at the code, and there are probably lots more things that could be improved, if you want to contribute.

ngasull commented 3 years ago

Great news, will sure have a look, thanks!

jaredh159 commented 3 years ago

If anyone is interested, i've got an alpha build of the new version ready for experimentation only -- not ready for production.

WIP documentation can be found here:

https://github.com/jaredh159/tailwind-react-native-classnames/tree/v2#readme

oste commented 3 years ago

@jaredh159 when testing the latest version in next.js react native web I am getting

error - ./node_modules/@jaredh/twrn/dist/index.js:1:0
Module not found: Can't resolve 'tailwindcss/resolveConfig'

Import trace for requested module:
./lib/tw.ts
./pages/index.tsx

https://nextjs.org/docs/messages/module-not-found
oste commented 3 years ago

yarn add tailwindcss seems to fix

jaredh159 commented 3 years ago

yes, tailwindcss is a peerDependency of the library now. you should have gotten a warning when installing (or, if you're like me, you've disabled all those warnings...)

Akumzy commented 3 years ago

Hi @jaredh159 I noticed there is no flex-1 in the v2, did you forgot or is there any plan for that?

jaredh159 commented 3 years ago

@Akumzy you know, I initially read from here that RN flex doesn't map to the web api, so I thought I couldn't support it. But I looked more closely at the tailwind classes and I see now that I can probably just break apart the shorthand into a combination of flexGrow, flexShrink, and flexBasis. Thanks for bringing this up, I'll get this implemented in v2 shortly. πŸ‘

jaredh159 commented 3 years ago

UPDATE 9/18 Hey, if you already installed the v2, I changed the npm org. Sorry for the churn, get back up and running with these commands:

npm uninstall @jaredh/twrn
npm install @jaredh159/twrn@next

The code in the comment above is updated, and correct.

Akumzy commented 3 years ago

@jaredh159 I'll give it a try, thanks

electroheadfx commented 3 years ago

thanks

jaredh159 commented 3 years ago

@Akumzy flex-1 and other flex shorthand utilities implemented in 2.0.0-alpha.12

jaredh159 commented 2 years ago

2.0.0 is released, which supports dark mode. thanks for the input πŸ‘