accordproject / cicero-ui

A library of React components for Accord Project templates
Apache License 2.0
29 stars 46 forks source link

refactor(*): replace style props with CSS classNames - #320 #387

Open 98lenvi opened 4 years ago

98lenvi commented 4 years ago

Signed-off-by: Lenvin Gonsalves lenvin@oykuapp.com

Issue #320

The goal is to remove all style props in favor of CSS class names. One pro I can think of is the user will have more flexibility on the appearance of the component,

Changes

Flags

Related Issues

98lenvi commented 4 years ago

@arteevraina. Tagging you here so you can get notified when this PR gets merged.

98lenvi commented 4 years ago

Can we also give the highest parent of each component a common class name? maybe ciceroUI? That way we can easily target specific elements within the ciceroUI parent.

I'd also like an explanation of the CustomStylesWrapper that you've added for each component. Thanks!

@DianaLease Yes I'll work on adding a common className. Thank you for the feedback

When I first added them they meant sense to me as it is one of the best way for me to added custom CSS style to elements. But now it doesn't seem required at all since the user will be adding his/her own wrapper around the component or style document. Sorry for the inconvenience, I'll be removing it!

DianaLease commented 4 years ago

@irmerk I think it makes sense to have a common className at the top level for all cicero-ui components. Like how semantic-ui adds ui as a class name to their parent components. That way when we apply styles we can limit those styles to elements within the .ciceroUI hierarchy.

irmerk commented 4 years ago

Following up on this @98lenvi, need any assistance?

98lenvi commented 4 years ago

@irmerk, actually my laptop's hard drive died a week ago. I'm waiting for the new drive to arrive (most probably tonight or tomorrow morning). I'll do the changes tomorrow morning! Thanks for asking!

irmerk commented 4 years ago

Also note the proposed changes in https://github.com/accordproject/cicero-ui/pull/411

irmerk commented 4 years ago

We're migrating this repository into web-components the next couple of days, so I'll be closing this soon and you should reopen there.