aeternity / aepp-components

deprecated: aepp-components to be used in all aepps.
ISC License
41 stars 14 forks source link

Feature/external global styles #194

Closed sadiqevani closed 5 years ago

sadiqevani commented 5 years ago

Splitting Global styles into:

'aepp.global': './src/styles/index.scss',
'aepp.fonts': './src/styles/_fonts.scss',
'aepp.components': './src/index.js'

->

Also updates in here are:

sadiqevani commented 5 years ago

Why you decided to use natural rem instead of rem()? After this change, a developer will have to manually convert pixels amount from Zeplin to rems?

So, in your pull-request for ae-button-group you were using natural rem, instead of the function, and I think it makes the code better.

Generally in the Sketch file, most things are defined in rems, and I'm also thinking of having a standardised REM variables, so spaces don't change in the long term.

If I will use just aepp.components.css without global styles, this won't break components from this package? Or I should fix them somehow by myself?

This won't break any global styles, the only problem I can foresee is that you will need to include fonts, as a secondary css.

Also I'm already using this branch in the aepp-token-migration repository.

sadiqevani commented 5 years ago

I don't agree to remove rem function while Zeplin shows all sizes in pixels, I don't want to manually convert them.

You can still use the rem() function, though for now I'm going to carry on and use only clean rem, this does not affect much of your work in the base-app, and its not that difficult to divide px amount / base-px

davidyuk commented 5 years ago

its not that difficult to divide px amount / base-px

And if I want to compare styles with designs I should multiply rems amount on base size? Very convenient!

We have other developers that working with components repository according to Zeplin mockups and we shouldn't convert rem(px) to rems until Zeplin shows all sizes in pixels. Stop making useless changes.