add ripple loader component from trendkite-ui

[lihnick]

No description provided.

[lihnick]

sm, md, lg are the sizes copied over from trendkite-ui

[lihnick]

Need a second opinion on spreading custom props in the nested div rather than the wrapper div.
Attaching it to the nested div allows consumer to pass in custom sizes and have it override the default sm, md, lg, xl options.
An alternative option would be to pass a numeric to the size prop rather than enum strings. Then the custom prop can be attached to the wrapper div. This will break away from the enum string pattern used in trendkite-ui

[pixelbandito]

I don't think I'd call this an "icon", I might just say "Loader" or something, since our icons are usually svgs that you can change colors on.

If the circular image ends up being its own component, it would probably be called Loader, but if it's nested inside a loader component and it would need a distinct name, maybe image, animation, ripple, or something.

[lihnick]

Updated the loader to render with one div and custom props would get spread on that div.
I kept the css that renders the loader's location in the center of its parent html element.
That way consumers can adjust the loader's horizontal positioning with a parent html element that has a defined width and margin.
They could also override the default horizontally centered loader position via style prop if consumers do not have control over the parent html element.

[pixelbandito]

At a high-level, I think the structure of this component in tk-ui isn't super clean, and we should take this opportunity to make a change.

The main thing that bothers me is something you already mentioned. It's weird that the loader props have to be spread on a child.

One of the principles we've been following is that components should affect layout outside themselves. Technically, the loader doesn't violate that rule, but it's weird that the actual circle animation automatically comes with a full width div and centering behavior.

My proposal would be:

• Make a component for the circular bit (the "loader"), that isn't full width. This component would just be the inner div, and can have normal props spread.
• Make a component called CenterLoader that wraps the base loader in a div with centering styles. That div could accept a loaderProps object that gets spread on the nested loader component.

Looking at Bootstrap and React Material-UI, it looks like they both have components for just the circular bits.

[pixelbandito]

This will also need to be re-exported from the main src/index.js file.
And you'll want to add a usage to the example/src/App.js and verify that it works over there.
(There's info about running the example app in CONTRIBUTING.md, but it's easy to miss.

[pixelbandito]

I don't think I'd call this an "icon", I might just say "Loader" or something, since our icons are usually svgs that you can change colors on.

If the circular image ends up being its own component, it would probably be called Loader, but if it's nested inside a loader component and it would need a distinct name, maybe image, animation, ripple, or something.

[pixelbandito]

With CSS modules, we usually follow a couple of slightly different naming conventions, since we don't need to worry about style collisions.

• The main class for the outermost DOM element wrapper of the component uses the same PascalCase name as the component itself, i.e. .Loader
• Child elements use camelCase class names, and don't need to be prefixed with the component name, i.e. .icon (although I also mentioned choosing a different name for this).
• Levels and sizes generally don't need to be prefixed (i.e. just .sm), although I'm not sure if this will scale for components that have lots of levels, sizes, states, etc.

[pixelbandito]

Super minor / non-blocker: I think .loader should still be .Loader

[pixelbandito]

One more small thing - for the "Overview" stories, we usually instrument all the documented props with "knobs".

Have a look at the Button story for examples if you want:
https://github.com/cision/rover-ui/blob/master/src/components/Button/story.tsx#L42

[pixelbandito]

Super minor / non-blocker: I think .loader should still be .Loader