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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How can i get primary or secondary color? #65

Closed
eder3232 opened this issue Sep 7, 2023 · 12 comments
Closed

How can i get primary or secondary color? #65

eder3232 opened this issue Sep 7, 2023 · 12 comments
Labels
request New feature or request

Comments

@eder3232
Copy link

eder3232 commented Sep 7, 2023

When you want to access the colors there is no way to select the primary or secondary color, like this:
<Heading color="text-primary-content background-neutral-200"> Trabajo final de estructuras de datos y algoritmos </Heading>
It would be good if they take Daisyui as a reference for the topic of primary, secondary, neutral colors, etc.

@needim
Copy link

needim commented Sep 7, 2023

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.

@vladmoroz
Copy link
Contributor

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"?

@needim
Copy link

needim commented Sep 14, 2023

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

@vladmoroz
Copy link
Contributor

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.

@needim
Copy link

needim commented Sep 14, 2023

Agree 👍

@vladmoroz vladmoroz closed this as not planned Won't fix, can't repro, duplicate, stale Sep 26, 2023
@toniopelo
Copy link

@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 :)

@vladmoroz
Copy link
Contributor

vladmoroz commented Oct 6, 2023

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.

@jd-carroll
Copy link

jd-carroll commented Oct 6, 2023

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

@jd-carroll
Copy link

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

@goerlitz
Copy link

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.

@vladmoroz
Copy link
Contributor

vladmoroz commented Oct 30, 2023

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.

@vladmoroz vladmoroz added the request New feature or request label Jan 4, 2024
@flyon
Copy link

flyon commented Apr 18, 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants