Open rednaksi91 opened 1 year ago
BSFishy
commented
1 year ago What we could do is make the component extend its child components, like we do in the title component: https://github.com/opensearch-project/oui/blob/d03ad49f67fff257f094b2ddad4832c7665125c4/src/components/title/title.tsx#L84
That would make it follow the same patterns that we already have in the project while giving the builder more flexibility.
BSFishy
commented
1 year ago
- Overriding existing styles: If the raw HTML already has some styling applied to it, applying a wrapper that overrides those styles can result in unexpected and unintended changes. This can make it difficult to maintain and debug the code.
It's possible to override OuiText styles without any issues: https://codesandbox.io/s/eloquent-roman-xh8196
- Lack of flexibility: Applying a wrapper that applies standard typography styling and spacing to raw HTML can limit the flexibility and customization options for individual components or pages. For example, if a particular page or component requires a different font size or line-height, it may be difficult to achieve that using the standardized wrapper.
This is the idea of creating a design system. We define an opinionated style for all of these components. Different font sizes can be achieved with OuiTitle or potentially other components.
- Increased code complexity: Adding an additional layer of markup can increase the complexity of the HTML and CSS code, which can make it harder to maintain and update in the long term. This can also impact the performance of the website or application.
I would argue that allowing the builders to submit a custom HTML tag increases complexity because it introduces another layer that OUI doesn't control. We could potentially run into situations where builders are getting strange functionality because they do something like <OuiText><table></table></OuiText> (not saying that this would be an issue, just an example of a potential edge case).
However, to your point, OUI injecting its own tag could also pollute the DOM. I think my previous comment addresses how we can get the best of both worlds.
- Accessibility issues: If the wrapper does not properly follow accessibility guidelines, it can lead to issues for users with disabilities. For example, if the font size is too small or the line-height is too tight, it can be difficult for users with visual impairments to read the content.
We make sure all of our components hit our accessibility targets. That is something we have committed to maintaining and something builders should never have to worry about.
I am aligned on allowing builders to specify their own tag, like I mentioned in my previous comment as long as @KrooshalUX is aligned.
KrooshalUX
commented
1 year ago I don't think this is the right change to make for OUI at this stage.To @BSFishy point, we are trying to provide an opinionated design system and controlled styling outputs, and will continue to do so at the design system level.
The current typography system we have is serving its purpose, and the UX team has yet to encounter any issues with needing additional styles. For context - any current overwritten or custom styles within the OSD product or plugins stem from issues before we forked OUI, or a lack of UX involvement in feature design (bias for action projects) or a miscommunications in designs before our Figma component library was published (ex: engineers implementing exact matches to mockups that were not intended as hifidelity artifacts).
Is your feature request related to a problem? Please describe. The EuiText component in OUI is a flexible and customizable text component that allows developers to set various properties such as color, size, weight, and alignment. It also supports responsive typography, which is an important feature for designing interfaces that work well across a range of devices and screen sizes.
However, EuiText is a wrapper that applies standard typography styling and spacing to raw HTML. This approach can be useful for applying consistent typography styles throughout an application, but it can also lead to several issues:
Overriding existing styles: If the raw HTML already has some styling applied to it, applying a wrapper that overrides those styles can result in unexpected and unintended changes. This can make it difficult to maintain and debug the code.
Lack of flexibility: Applying a wrapper that applies standard typography styling and spacing to raw HTML can limit the flexibility and customization options for individual components or pages. For example, if a particular page or component requires a different font size or line-height, it may be difficult to achieve that using the standardized wrapper.
Increased code complexity: Adding an additional layer of markup can increase the complexity of the HTML and CSS code, which can make it harder to maintain and update in the long term. This can also impact the performance of the website or application.
Accessibility issues: If the wrapper does not properly follow accessibility guidelines, it can lead to issues for users with disabilities. For example, if the font size is too small or the line-height is too tight, it can be difficult for users with visual impairments to read the content.
Describe the solution you'd like In contrast, Material UI (MUI) uses the Typography component, which supports an inline variant prop that maps to a range of different HTML element types. This allows developers to apply different typography styles to different parts of the text without having to create separate wrapper components for each style. This can make development more efficient and clear.