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

Only have types for data-color and data-size when designsystemet type is added to users tsconfig #2943

Open
1 of 2 tasks
mimarz opened this issue Jan 3, 2025 · 3 comments
Assignees
Labels
🐛 bug Something isn't working react @digdir/designsystemet-react

Comments

@mimarz
Copy link
Collaborator

mimarz commented Jan 3, 2025

Suggestion for improvement.

Currently if you have not added color.d.ts or react-types.d.ts, usage of data-color and data-size is quite inconsistent and confusing.

Depending on where you are using the data-attributes you get different types.

Confusing aspects
- Used on plain elements (div, span), you can not use any string, which you can on other data attributes.

  • Used on one of our components, you get just half of the types (severity colors). This makes it look like these are all the available types, when in fact there is more.

Suggestions

Tasks

Preview Give feedback
@mimarz mimarz converted this from a draft issue Jan 3, 2025
@mimarz mimarz changed the title Only have types for data-color when type is added to users tsconfig Only have types for data-color and data-size when designsystemet type is added to users tsconfig Jan 3, 2025
@mimarz mimarz added the react @digdir/designsystemet-react label Jan 3, 2025
@mimarz mimarz added this to the V1 - Must-have milestone Jan 3, 2025
@mimarz mimarz added the 🐛 bug Something isn't working label Jan 3, 2025
@Barsnes
Copy link
Member

Barsnes commented Jan 3, 2025

I think the default type should contain what it does today, but also string, so it accepts anything, but you also get suggestions.

@mimarz mimarz moved this from 🔵 Inbox to 📄 Todo in Team Design System Jan 3, 2025
mimarz added a commit that referenced this issue Jan 8, 2025
#2951)

#2943

- Made it so `data-size` and `data-color` also accept any string as
default so its the same as behaviour for other data attributes.
  - Except for explicits components
- Removed references and default values for `data-size` and `data-color`
in stories as thats contextual.
- Removed unnecessary override to `data-size` and `data-color` to make
sure the jsdocs and behaviour is uniform.
- Added `data-color` and `data-size` as global `argTypes` for storybook
so that they show up in controls for all components
@mimarz mimarz moved this from 📄 Todo to 🏗 In progress in Team Design System Jan 17, 2025
@mimarz mimarz self-assigned this Jan 17, 2025
@mimarz mimarz moved this from 🏗 In progress to 📄 Todo in Team Design System Jan 17, 2025
@unekinn
Copy link
Contributor

unekinn commented Jan 17, 2025

One solution is to fallback to string | {} when the type hasn't been extended. This ensures developers only get autocomplete suggestions if the type is "completed" via a colors.d.ts file. I think this should be possible using IsEmptyObject from type-fest

In packages/react/src/colors.ts, use this:

import type { IsEmptyObject } from 'type-fest'

export type Color = IsEmptyObject<MainAndSupportColors> extends true
  ? string & {}
  : CustomColors | SeverityColors;

If we don't want to depend type-fest, we can inline the definition:

declare const emptyObjectSymbol: unique symbol;
type EmptyObject = {[emptyObjectSymbol]?: never};
type IsEmptyObject<T> = T extends EmptyObject ? true : false;

Example: When empty

Image

When extended:

Image

@unekinn
Copy link
Contributor

unekinn commented Jan 17, 2025

Actually, maybe this is better, since some of our components import just CustomColors or SeverityColors

type ColorWithFallback<T> = IsEmptyObject<MainAndSupportColors> extends true ? string & {} : T

export type SeverityColors = ColorWithFallback<'info' | 'success' | 'warning' | 'danger'>

export type CustomColors = ColorWithFallback<'neutral' | keyof MainAndSupportColors>

export type Color = CustomColors | SeverityColors;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working react @digdir/designsystemet-react
Projects
Status: 📄 Todo
Development

No branches or pull requests

3 participants