SAP / fundamental-react

React implementation of the reusable component library designed in Fundamental Library Styles
https://sap.github.io/fundamental-react
Apache License 2.0
189 stars 78 forks source link

Migrate components to styled-components #139

Closed derberg closed 4 years ago

derberg commented 5 years ago

Description:

Reason:

When building an application you might need to use only few components and instead of including globally a stylesheet with styles for the whole components library doesn't make sense.

Hint To quickly check how styled-components work take a look on one of ours component https://github.com/asyncapi/asyncapi-react and quickly play with it https://codesandbox.io/s/5vz8l9zlmn to see how theming works and that in case of styled-component for global stylesheet you need only a font reference

derberg commented 5 years ago

@magicmatatjahu how do you think? what would be the best approach for doing the styled-components. When we did ours in Kyma, case was simple, we had our css and tried to be compliant with fiorii 3. So CSS was always "manually" added to the component itself like here https://github.com/kyma-project/console/blob/master/components/react/src/components/Button/index.js

With Fundamentals we would not really like to duplicate CSS right? to have easy bumping of the version in case of new Fundamental releases. Best would be to reuse the npm package they deliver with CSS and Sass files https://github.com/SAP/fundamental#npm. That will be tricky I guess

magicmatatjahu commented 5 years ago

@derberg The styles downloaded from the package is a good option. At the moment I do not have any better solution.

A few comments regarding writing styled components:

magicmatatjahu commented 5 years ago

Done:

derberg commented 5 years ago

@magicmatatjahu would be awesome if you could open the WIP PR from the very beginning so the library owners can see what changes are you introducing

bcullman commented 5 years ago

It looks like for this item, style would be served directly from this repo. I think we need to decide NOW what our relationship is with https://github.com/SAP/fundamental.

1) reference https://github.com/SAP/fundamental is what we should aspire to, but each fundamental-[X] repos (react/vue/ngx) is free to write their own CSS

2) dependency use CSS from https://github.com/SAP/fundamental directly, all fundamental-[X] repos (react/vue/ngx) are required to emit the same HTML, but left to their implement in the manner best suited to that framework.

Personally, I vote for dependency.

Additionally, during transition, SAP Concur will require the ability to use these components "style-less" (we will provide our own style)

derberg commented 5 years ago

@bcullman I think approach other then dependency doesn't make sense because then we are not fundamental-react but fundamental-like-react :)

and as stated in description, each component will have theme props that you can use to fully override default styles

magicmatatjahu commented 5 years ago

With @derberg we decide to write node scripts to load css from https://github.com/SAP/fundamental (before building prod or dev env), parsing this css to object { selector: cssValuesAsString } and then load this css to styled components:

import styled from 'styled-components';
import tokenStyles from /* preval(["fiori-fundamentals/dist/components/token.css"]) */ '../common/preval-css'; // we use preval to run node script before building env

export const TokenWrapper = styled.span`
    ${tokenStyles}
`;

@bcullman What are you thinking about this? With that we will have dependency relationship and also reference with option to fully override default styles.

bcullman commented 5 years ago

You should not need to write a node script for this - there are webpack plugins that can do practically any transform you want.

I would like to look at supporting (or at least discussing supporting) the following support scenarios. There may be more.

1) use react to load a fundamental-react component on a page that already has the full fiori fundamental CSS declared in the page template

1) load a fundamental-react component on a page that injects it's own styleblock for just it's own component on to a page that already has the minimum fundamental CSS (CSS reset, grid and font sizes) declared in the page template

1) load a fundamental-react component on a blank page that injects it's own styleblock for just it's own component on to a otherwise blank page.

1) load a fundamental-react component on a blank page with no CSS at all (Initially, SAP Concur will need this)

magicmatatjahu commented 5 years ago

@bcullman About what webapack plugins are you thinking? CSS-loader or Extract-Text? I hope that you know, that CSS-loader only refer to class/selectors and add css to bundle, not extracting values from class/selectors (maybe exists some option to this plugins for that, but I made research and I didn't found any solution).

To styled components we must pass css values (by class SC doesn't work) and at the moment doesn't exists library/plugin to simple split css class to component (by block, element and modifier) - I searched some solutions by 2 days and nothing found, so at the moment this task is impossible to done.

derberg commented 5 years ago

@bcullman looks like doing what we want with styled-components is not possible. AFAIK @magicmatatjahu contacted directly its maintainers and they also confirmed that without writing any custom library to support our approach it is not possible. Looks like the only option might be css modules, but we do not have much experience here. I think some time ago you mentioned you explored that topic a bit. How do you think, better now to stop styled-components and you could maybe asses css modules in depth or what are the other options in your opinion?

bcullman commented 5 years ago

Our team is happy to take a look at CSS Modules, and we do considering it a High Priority item to look at.

That said, right now, it is higher priority for my team get though the tasks and issues listed Production-ize fundamental-react board here: https://github.com/SAP/fundamental-react/projects/2.

Once those are completed, and we start consuming some of the simpler components, we will circle back on CSS Modules.

mobot11 commented 5 years ago

https://medium.com/nulogy/how-to-use-css-modules-with-create-react-app-9e44bec2b5c2 Helpful article on how to implement with Create React App.

derberg commented 5 years ago

Ok, so let us part topic of styled-components until you do your research on CSS Modules. Then let us sync on what is the final approach we take, which is the best and what additional contributions it requires so we can join forces in case it is complicated.

For now we will just focus on extending/contribution to components with additional props if needed.

jbadan commented 4 years ago

We decided to go a different direction with styles.