mantinedev / mantine

A fully featured React components library
https://mantine.dev
MIT License
26.87k stars 1.9k forks source link

Support custom spacing for Stack #1712

Closed ivan-kleshnin closed 2 years ago

ivan-kleshnin commented 2 years ago

What package has an issue

@mantine/core

Describe the bug

In our team we prefer explicit rem-based constants over xs | sm | md | .... They are more obvious and flexible. Also it's sometimes necessary to use more distances like 0.5rem | 0.75rem | 1rem | 1.25rem | 1.5rem | 2rem that no reasonably short list of allowed units would be able to contain.

Now some component props support custom dinstances and some don't:

<Stack mt="1.5rem"> -- supported
<Stack spacing="1.5rem"> -- not supported (in TS) 

For some reason, module augmentation doesn't work either:

declare module "@mantine/core" {
 interface StackProps {
    spacing?: string // or MantineStyleSystemValue
  }
}

export {}

  1. Not sending a PR because I'm not sure it's not "by-design" (I'd argue but it's not my library).
  2. It's a purely type-level issue, works correctly in JS...

In which browser did the problem occur

Any

If possible, please include a link to a codesandbox with the reproduced problem

No response

Do you know how to fix the issue

Yes

Are you willing to participate in fixing this issue and create a pull request with the fix

Yes

Possible fix

export interface StackProps extends DefaultProps, React.ComponentPropsWithoutRef<'div'> {
-   spacing?: MantineNumberSize;
+   spacing?: MantineStyleSystemValue;
    align?: React.CSSProperties['alignItems'];
    justify?: React.CSSProperties['justifyContent'];
}
rtivital commented 2 years ago

Mantine components are built with px. I recommend you to also use px to keep things consistent. For example, <Stack spacing={20} />

ivan-kleshnin commented 2 years ago

Ok, point taken. Is there a blog post or a documentation page that would clarify that? Something that I could refer other people to...

You know, there's a huge # of developers who are convinced that pixel values should never be used. And it's like a pendulum swinging between "never" and "always" each couple of years. So many of us have lost the track of the current "best practice" here 😞

rtivital commented 2 years ago

Nope, we do not have any additional documentation for that. It is just how Mantine is built, it is usually described in components props table.

ivan-kleshnin commented 2 years ago

No problem. Thank you for answering and, of course, for making and publishing this library 🤝 The quality of docs, UI look, and code is top-notch. Keep up!