Code Monkey home page Code Monkey logo

Comments (12)

toniopelo avatar toniopelo commented on May 13, 2024 5

@vladmoroz That's a shame that "accent" is not part of the possible values in the color enum.
I was actually going to create an issue for that, but as it's discussed here already, I'll just comment.

I get that this seems to be messing with your mental model a little bit (semantic vs literal and all) but it's really what feels the most natural from a DX point of view (IMO).
Plus, the workaround proposed with the useThemeContext().accentColor would force you to use a client component and react context where it is absolutely not needed. You could get away with a server component and a static "accent" value. It would also create a "blink" if you use SSR I guess, didn't test.

I would also argue that the value of the Theme provider itself is to be able to change a single props and all your components gets updated according to your new theme, so saying that a search&replace is the way to go kind of defeat the whole purpose of theming feature don't you think ?

In my case, I need to have some <Heading color="accent"></Heading> components colored with the accent color sometimes, and it feels wrong to have to do some tricks to have it connected to the theming feature. I use tailwind with the radix preset and I do <Heading className="text-accent"></Heading> for now, which yield the intended result.
But this approach is not consistent across the app, as sometimes you would use tailwind, sometimes the color prop, and for non-tailwind user there is just no way to achieve this without the workaround.

I don't want this comment to feels like a bad appreciation of the tool, Radix is a great tool that I'll probably use on my projects from now on, I just wanted to add my two cents to the discussion and see where it goes :)

from themes.

needim avatar needim commented on May 13, 2024 3

For example, you give the ability to change the accent color to the user. So you don't know what the accent color really is.

the current workaround for this:

import { useThemeContext } from "@radix-ui/themes";

const currentAccentColor = useThemeContext().accentColor;

<Heading color={currentAccentColor}>hello</Heading>

Another example: maybe at some point, I wanted to change the accent color to another one. So I need to find & replace all violet with blue.

Related: #33

from themes.

vladmoroz avatar vladmoroz commented on May 13, 2024 1

For example, you give the ability to change the accent color to the user. So you don't know what the accent color really is.

Right, this is a bit of niche case for many apps though and as you said, there is an easy workaround for it. We are aware that it is super common to use a keyword for the accent in component libs, but we don't want to start mixing semantical keywords with literal values as much as possible. The other risk is that this invites an argument for other keywords, like "secondary", "primary", "tertiary", etc., which dilutes the API.

That's why I am curious about the practical problem at hand that @eder3232 experiences

Another example: maybe at some point, I wanted to change the accent color to another one. So I need to find & replace all violet with blue.

This is a big change design-wise and a plain find/replace seems manageable; you wouldn't have to use the accent keyword most of the time anyway as it's the default.

from themes.

jd-carroll avatar jd-carroll commented on May 13, 2024 1

Not to belabor this, but... (please read)
-> I do not think accent should be part of the "API"

Wouldn't the API be better off not providing any colors?

There are some odd 26 colors right now. But that's it. If you wanted to create your own custom palette, you would essentially need to hijack a color you don't think you'll need and override all the color variable definitions.

I think the only colors that should exist out of the box are: black, white, gray, accent

Again, this is not to diminish the auto gray matching or any of the other features that work out of the box. Those are all amazing. But from a component API perspective, I would call them unnecessary.

Rather, I would propose the following: (see update in next comment)

@radix-ui/themes
  1. The only colors included are: black, white, gray, accent
  2. The default accent color is builders choice (something distinctive to the radix team)
  3. All gray auto matching and any other color specific logic is removed
@radix-ui/themes-palette
  1. A new color specific module is created and is a dependency of @radix-ui/themes with the goal of being a transparent change (non-breaking) when released
    (-> could also make the case for this to be a breaking change and may make the implementation cleaner)
  2. The new module will expose a ThemePalette context but not implement any provider. The context will provide the same color specific helper methods removed from @radix-ui/themes
  3. The main module (@radix-ui/themes) will look for this context any time it wants to interact with colors. If none is found, the main module can reference a static default implementation with all the existing colors.
  4. Applications will be able to implement a provider for this context to include any custom colors they want (falling back to the static default implementation)
Solving Typescript types
  1. The new palette module will expose a PaletteColor type
export interface PaletteColorOverrides {}

export type PaletteColor = OverridableStringUnion<
  'black' | 'white' | 'gray' | 'accent',
  PaletteColorOverrides
>;
  1. Applications can provide their own colors by writing:
declare module @radix-ui/themes-palette {
  interface PaletteColorOverrides {
    AcmeCorp: Palette;
  }
}

(may not be exact, but is the same concept from MUI)

Solving third-party component libraries
  1. The component library should expose what colors it expects as part of its API.
  2. The colors defined should be semanitcal, i.e. error, accent, info, etc. and not the names of specific colors
  3. As the component consumer, I would then provide a mapping of those semantical colors to the colors available in my applications palette. (IMO a component auto-magically picking up the colors that it "knows" exist is an anti-pattern)
  4. NOTE: This mapping can be done in a custom ThemePalette context provider the application builder writes. And the actual color names in the component should probably be specific to the component, something like Cool-Grid-Primary.
  5. Also, a third-party component developer could write their own ThemePalette context provider

Further, with this scheme if an application team wanted color names like primary, info, warn, secondary, etc. then they have every opportunity to implement those.

I doubt this is the most elegant solution, but I think this accomplishes two specific goals:

  1. Distill the core API to only the necessary elements (aka. black, white, gray, and accent)
  2. Provide an extensible framework for application development teams to provide the colors that are meaningful to them

from themes.

goerlitz avatar goerlitz commented on May 13, 2024 1

Hey, I came here as well while looking for a way to use primary, secondary and semantic colors (success, warning, error, info), like in Material and Bootstrap.

Do I get it right that Radix does not support it? Just one accent color?

Actually, it is a common design thing to define your brand colors (corporate identity) with a color schemes that includes primary, secondary, neutral colors to be used throughout your UI. (like described here). It would be really cumbersome to use only named colors all over the place, including issues with consistency and find/replace when you need to update your primary and secondary colors.

from themes.

needim avatar needim commented on May 13, 2024

I believe <Heading color="accent">hello</Heading> should work.

image

But if you are using TypeScript currently you will get an error.

image

This type should be fixed.

from themes.

vladmoroz avatar vladmoroz commented on May 13, 2024

I believe <Heading color="accent">hello</Heading> should work.

This would work in most cases indeed as it is a side-effect of the implementation, but we might look into doing it properly at some point. Right now we are not sure if it makes sense with the rest of the literal values in the color prop throughout the lib and how valuable it is.

Any reason you don't want to use a literal value, like color="violet"?

from themes.

needim avatar needim commented on May 13, 2024

Agree 👍

from themes.

vladmoroz avatar vladmoroz commented on May 13, 2024

So, so far the arguments for an accent value are:

  • To make it easy to swap accent colors on the components that aren't using accent color by default.
  • To improve DX ergonomics and minimise re-renders for when accent color is configurable by the end user, again just on the components that aren't using accent color by default.

It's a reasonable, even conventional request, but both cases still seem too fringe to warrant the API compromises, which we value strongly in its current form. It's not often that you change an accent color in your project, and user-configurable accents are even more rare in the kind of projects that Radix Themes is geared for.

That said, there are also workarounds if either case is important to your particular situation—you can create a plain string constant with your literal accent color, or extract it from the theme context if you need a dynamic value.

As a general rule, we don't optimise the API for rare cases that have straightforward workarounds.

I would also argue that the value of the Theme provider itself is to be able to change a single props and all your components gets updated according to your new theme, so saying that a search&replace is the way to go kind of defeat the whole purpose of theming feature don't you think ?

No, not really, all “colored” components default to the accent color, so the value of the Theme provider isn't nearly defeated. For the handful of components that don't use accent by default, I think that find and replace is easy enough for the few times you might change it throughout the life of your project.

I do for now, which yield the intended result.

I'd really recommend to stick to the API we have and give literal values a try 🙂
We make sure that you get automatic colors on nested components, like Link, Code, etc., auto selection color, and you can be sure that it will be compatible with newer versions.

from themes.

jd-carroll avatar jd-carroll commented on May 13, 2024

Having thought on this for 5 minutes... This should be a breaking change.

Some edits:

@radix-ui/themes
  1. The only colors included are: black, white, gray, accent
  2. The default accent color is builders choice (something distinctive to the radix team)
  3. All gray auto matching and any other color specific logic is removed
  4. A new ThemePalette context is exposed but no default provider is implemented. The context will provide the same method signatures as those removed.
  5. The Theme will look for a ThemePalette provider any time there is an interaction with colors. If no provider is found, then Theme will fallback to the default colors black, white, gray, accent.
@radix-ui/themes-palette
  1. A new color specific module is created. This would be separate from the main module and therefore a breaking change.
  2. All colors and color related logic removed from the main module will be moved here.
  3. Will provide an extensible implementation of a ThemePalette context provider.
  4. Applications will be able to implement their own ThemePalette provider extending from the default included in this module to add any custom colors they want

from themes.

vladmoroz avatar vladmoroz commented on May 13, 2024

Hey @goerlitz
We are perfectly aware that semantic colors is another common approach. I also had used and built component libraries like this, but moving away from it for all the reasons I mentioned above. We think that certain semantic names don't make sense for every UI, while some other UIs will have cases for more semantic name needs that might be unique just to them. On top of that, the products we've been building for the past few years have all used literal colors—we've been the happiest with this approach so far because of how simple it is.

I suggest that you try to give literal values a go.

If that still bugs you and you absolutely need semantic colors, you can always wire them up for yourself:

  • Create your own scales that define colors like --danger-1, --danger-2 based on the colors we provide
  • Create your own components that would wrap Radix Themes components, but provide a different API like color: "brand" | "subbrand" | "danger" | "etc" that may internally map to the corresponding literal colors.

from themes.

flyon avatar flyon commented on May 13, 2024

Just climbing in here to say that for an eco system of reusable Radix components it makes a lot more sense to be able to add custom colors like primary,secondary,tertiary then to only allow specific colors.

Because when components with specific colors get combined it gets messy. Whilst components made by multiple devs that all used 'primary' / 'secondary' would easily look well integrated by defining those colors

from themes.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.