Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shared React component library #76

Closed
5 tasks
olizilla opened this issue Mar 6, 2019 · 13 comments
Closed
5 tasks

Shared React component library #76

olizilla opened this issue Mar 6, 2019 · 13 comments
Assignees
Labels
topic/design-visual Visual design ONLY, not part of a larger UX effort

Comments

@olizilla
Copy link
Member

olizilla commented Mar 6, 2019

We want to reuse our UI componets across at least ipfs-webui, ipfs-desktop, ipfs-share-files. We want to define the patterns we have to creating our presentational components, in terms of component styling, and overrides, and i18n and bundling locale dictionaries when the componet is used in an app.

We believe we want a shared component library. A dev experience of

import { CidInfo, LoadingIndicator } from ipfs-gui

would be nice, epecially if it could automagically sort out adding the additional i18n keys to the bundle.

Prior art:

Puzzle pieces:

Next steps:

Starting from the gui-team principles... take a look at https://github.com/ipfs-shipyard/ipld-explorer-components

  • update storybook to the fancy new one.
  • add the circle ci build process to it. Have it publish the storybook in PR. Comment out the dnslink update step, until we decide on a domain for it.
  • Pull some components from ipfs-webui into ipld-explorer-components (will be renamed later) and import them from webui
  • Define the pattern for exporting re-usable components that use tachyons for styling, but allow the use site to overide the style of the containing element. This is probably just
export const Thing = ({className, children, ...props})  => {
  return <div className={`bg-gray charcoal ${className}` ...props>{children}</div>
}

to start with, but it has the problem that if ma4 is the default, and the caller tries to apply ma2 as an override it will not work as the class will end up with both in its class list, and then its down to document order in the css file as to which one wins, in this example, the default ma4 occurs after ma2 so it does not apply. I think we can figure out a neat solution to that tho.

  • Define the component api for i18n. Right now we export components that expect a t function and call that with i18n key names that they need. Alternatives are to create props for each label that appears in the component, and then write a container that maps those props to i18n keys. This could be neat if we use the path/to/the/Component.fooLabel as the key. In general you want to make it easy to have sensible defaults and possible to override them when the compent is used in a different setting. It's worth highlighting that really atomic components like <Button> should not have any awareness of i18n, as their text will change with every use case, but something like a <DeleteButtonWithConfirmation/> might benefit from sharing good default explanation text, that would need localising.
@olizilla
Copy link
Member Author

olizilla commented Mar 6, 2019

@olizilla
Copy link
Member Author

olizilla commented Mar 6, 2019

Also, it's worth noting that pulling components into a shared library can harm the dev experience if means you often have to npm link the component library into your app so you can fix the shared components to work in your new use-case. I learn towards avoiding making overly generic components in favour of sharing good composable basic ones. If a component ends up with numerous props to customise it, and internal logic to deal with the variety, splitting it out into 2 simpler components is usually easier to reason about.

Worth considering

  • https://bitsrc.io - makes a shared library feel like you can edit it locally without much friction. I love the idea but i fear other peoples magic. Would be worth trying out tho.

@GiladShoham
Copy link

Hi all, I’m Gilad and I’m a core maintainer at Bit. I will happily shed some light on Bit’s sorcery 🙂 I think it can be a useful solution to this problem. For example, here’s an example of what Grommet are in the process of doing with Bit: https://bit.dev/grommet.

As you can see, every component can now be reused in other projects and even developed from multiple projects at the same time while changes can be synced. All the components are available in the collection and can be shared, discovered and playd with. This will not require any refactoring for the library. You can learn more here: https://docs.bit.dev/whybit/for-libs.html. Alternatively you can even save the need for the library altogether and just share components from different places into one collection.

What do you think? I’d love to help

@olizilla
Copy link
Member Author

Thanks @GiladShoham we're testing Bit out! @cwaring @fsdiogo can you add your feedback here as you go?

@fsdiogo
Copy link

fsdiogo commented Mar 27, 2019

The learning curve seems quite low as the commands are all Git-like 👍

In a few minutes we had a collection with a few components, imported them from Bit and they worked out-of-the-box in our apps, so that is awesome 👌✨

A pain point we experienced is when we have dependencies higher up in the tree: we're using Tachyons and others and importing them in a global CSS file. So, to make our components "stylezed" in Bit, we've got to import those libs in each component on our collection (AFAIK). Is there a way to import a dependency globally to our collection?

@GiladShoham
Copy link

I'm happy to see that you find Bit easy to use :) It's humbling to see an awesome team liking it!

We are working right now on the ability to add dependencies globally. So you can add Tachyons as a dependency to all components (or a subset...). It is part of the overrides feature (that is already merged). I expect it to be available in the next couple of weeks.

I can update you on its availability here if you'd like.

@fsdiogo
Copy link

fsdiogo commented Mar 28, 2019

Awesome! Please do @GiladShoham!

@olizilla
Copy link
Member Author

Storybook is getting docs n blocks n mdx - https://medium.com/storybookjs/storybook-docs-sneak-peak-5be78445094a

@ericronne
Copy link

Any update on tachyons (&c) support, @GiladShoham? Thanks!

@cwaring
Copy link
Member

cwaring commented Apr 30, 2019

@ericronne and I kicked around some ideas on this today and concluded that Bit neatly helps us achieve the goals we have for the first version of our component library. So I'm going to run an extended test based on ipfs-webui soon 👌

@GiladShoham
Copy link

@fsdiogo @olizilla @ericronne @cwaring
I'm happy to hear you find Bit useful for your project :)

We've just published a new version for Bit (14.1.0). It supports a new workflow for manipulating Bit's automated dependency definition. It allows forcing a dependency to all components, even if they don't explicitly require it.

How to force a component dependency

I'll assume that you are already tracking the styles component, and name it styles-component.

Edit the workspace configuration and add the overrides key. Note that you can set it for specific components or groups of components.

{
  "bit": {
      "overrides": {
          "*": { // Add the css-component as dependency to all workspace components
              "dependencies": {
                  "@bit/css-component": "+" // bit defines the version automatically, as defined in the workspace
              }
          },
          "utils/*": { // if there are some components that don't need the dependency, you can use the override to remove dependencies
              "dependencies": {
                  "@bit/css-component": "-"
              }
          }
      }
  }
}

You can read a bit more about it here.

  1. short description
  2. Overriding component dependencies
  3. Setting specific component configurations

If you need any help, you can open an issue in the main git repo, comment to this thread, or say 'hi' on gitter.

@cwaring
Copy link
Member

cwaring commented May 1, 2019

Thank you @GiladShoham, this looks ideal. I'll test it out and let you know how we get on :)

@ericronne ericronne added the topic/design-visual Visual design ONLY, not part of a larger UX effort label Jun 28, 2019
@jessicaschilling
Copy link
Contributor

Closing in favor of #29 (and linking to this issue inside it as well, so work isn't lost).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/design-visual Visual design ONLY, not part of a larger UX effort
Projects
None yet
Development

No branches or pull requests

6 participants