Make Vulcan UI-library-agnostic

SachaG commented 6 years ago

Currently, Vulcan has a hard dependency on Bootstrap. We want to break this dependency to enable Vulcan to be used with other UI libraries, such as Material UI and Semantic UI.

Existing Dependencies

Currently, multiple places depend on Bootstrap in Vulcan's core packages:

vulcan:core

The vulcan:core package depends on react-bootstrap for its core components (modal, button, alert):

Button
Alert
Modal
- Modal.Header
- Modal.Title
- Modal.Body
- Modal.Footer

vulcan:forms

The vulcan:forms package depends on formsy-react-components for its form components (inputs, etc. as well as the form layout itself)

Checkbox
CheckboxGroup
Input
RadioGroup
Select
Textarea

Additionally:

DateTimePicker from react-datetime

example-forum (and other example packages)

In addition, example packages (such as the Forum example) also depend on react-bootstrap for dropdowns, buttons, etc.

Button
Dropdown
- DropdownToggle
- DropdownMenu
- DropdownButton
MenuItem
Alert
Modal
- ModalHeader
- ModalBody
- ModalFooter
FormControl

CSS

Additionally, using Bootstrap requires loading the Bootstrap CSS separately, either locally or via CDN>

Goals

Step 1: Core Dependency

Edit: Actually, this can come after Step 2

Given how limited the core dependency is, it would not be hard to remove any dependency on a library altogether.

[ ] Make ModalTrigger component work without Bootstrap
[ ] Make Button and Alert into generic non-Bootstrap core components

Note: this is optional, we could also go straight to Step 2.

Step 2: Component Middle Layer

~~Finally, in order to make examples UI library-agnostic, we could expand the current core components into an entire middle layer of components that themselves call Bootstrap/Material/etc.~~

Edit: this "middle layer" doesn't need to actually exist "physically". i.e. we don't need a Button.jsx file containing a component that calls a bootstrap version, it's enough to have each Bootstrap/Material/etc. theme register its own Button component.

In other words, instead of <Button/>, the examples would call <Components.Button/>. Instead of <Input>, <Components.Input/>, etc.

Eventually, that layer of components can optionally be extracted out of vulcan:core and given its own package.

Step 3: "VulcanUI" Components

Eventually, a third step could be adding a "native" implementation of each component, so that Vulcan has its own <Button/> that doesn't require any UI library.

This would be especially useful for forms, as even in a totally custom-made app that has no UI library dependency, you might still want to save yourself the pain of styling forms.

luhagel commented 6 years ago

I'll handle the core dependencies, that's a big win for people not using any of the example packages

justinr1234 commented 6 years ago

I don’t believe this is the proper solution. There are too many structural variances between component libraries to make this viable. The abstraction should be at the composite component layer:

NewPostButton -> [NewPostButtonBootstrap3, NewPostButtonSemantic, NewPostButtonMUI, NewPostButtonAnt, NewPostButtonBootstrap4]`

A perfect example is the difference between the bootstrap 3 React library react-bootstrap and the bootstrap 4 library reactstrap:

With react-bootstrap:

import { LinkContainer } from ‘react-router-bootstrap’;
import Button from ‘react-bootstrap/lib/Button’;

///

<LinkContainer to=“/“>
   <Button bsSize=“lg” />
</LinkContainer>

Using reactstrap:

import { Link } from ‘react-router’;
import { Button } from ‘reactstrap’;

///

<Button tag={Link} to=“/“ />

Extending the example further we’d have the equivalent:

registerComponent(‘NewPostButton’, NewPostButtonBootstrap3);
registerComponent(‘NewPostButton’, NewPostButtonSemantic);

justinr1234 commented 6 years ago

Another issue is that people purchase themes off different websites that are pure html/css. Crafting a new UI framework as in your #3 proposal is basically duplicating the work people have already done with UI frameworks and removes the ability to drop in a purchased css/html theme created using one of the frameworks. I think it is unlikely and likely not worth it for this project to maintain a proper abstraction layer of a UI component library when that problem is already being solved over and over by full time libraries like Bootstrap, MUI, Semantic, etc.

MHerszak commented 6 years ago

Thanks @SachaG, this is a very good overview. I have a different approach in mind, I have not yet worked with Gatsbyjs, yet, but would it be possible to integrate Vulcan with Gatsby? My gut tells me, building packages for several component libraries isn't the right approach (just my gut not data to underline this). For instance, my reactstrap implementation for modalTrigger is this =>

import { replaceComponent } from 'meteor/vulcan:lib';
import React, { PureComponent } from 'react';
import PropTypes from 'prop-types';
import { Modal, ModalBody, ModalHeader } from 'reactstrap'

const propTypes = {
  className: PropTypes.string,
  label: PropTypes.string,
  component: PropTypes.object,
  size: PropTypes.string,
  title: PropTypes.oneOfType([PropTypes.string, PropTypes.element]),
};

const defaultProps = {
  size: 'large'
};

class ModalTrigger extends PureComponent {

  static propTypes = propTypes;

  static defaultProps = defaultProps;

  state = {
    modalIsOpen: false
  };

  toggle = () => {
    this.setState({
      modalIsOpen: !this.state.modalIsOpen
    });
  };

  renderHeader() {
    return (
      <ModalHeader>
        {this.props.title}
      </ModalHeader>
    )
  }

  render() {
    const triggerComponent = this.props.component ?
      React.cloneElement(this.props.component, { onClick: this.toggle }) :
        <a href="#" onClick={this.toggle}>{this.props.label}</a>;

    const childrenComponent = React.cloneElement(this.props.children, { closeModal: this.toggle });
    return (
      <div className="modal-trigger">
        {triggerComponent}
        <Modal
          className={this.props.className}
          size={this.props.size}
          isOpen={this.state.modalIsOpen}
          toggle={this.toggle}
        >
          {this.props.title ? this.renderHeader() : null}
          <ModalBody>
            {childrenComponent}
          </ModalBody>
        </Modal>
      </div>
    )
  }
}

replaceComponent('ModalTrigger', ModalTrigger);

replaceComponent does a good job already. However, I don't believe it is an advantage to build for "every" component library (I don't have a better approach) because you will add more work to keep up with all the new developments. Here is just a thought, a platform itself is successful when you create tools for devs to build tools for the platform. However, this might look like. A good idea, which isn't implemented, yet, was the module idea which wasn't ported from Telescope to Vulcan. Those modules, registerTemplate, and registerComponent would already be very powerful. Having said this, the mentioned "component middle layer" might already be enough for Vulcan.

To conclude my none negative comment, I drifted a bit away from Gatsby :D. Does anyone have experience with Gatsby? Would Gatsby make sense for Vulcan?

justinr1234 commented 6 years ago

@SachaG I finished a proof of concept of my idea: https://github.com/VulcanJS/Vulcan/compare/master...justinr1234:vulcan-new-ui?expand=1

Check it out, then either swap between bootstrap3 or bootstrap4 or semantic in your settings.json

The only components updated are PostsNewButton and ModalTrigger (that PostsNewButton relies on).

The last argument of registerComponent can optionally accept an options object of the form { framework: 'bootstrap4', theme: 'mytheme' }
Not specifying an options object will register that component for the theme selected in settings.json by default
Stylesheets are registered inside vulcan:core and loaded on demand depending on the current framework selected in settings.json. For this example, I am swapping between a newer bootsrap 4 and an older bootstrap4 CSS that is still defined in Vulcan.
It's backwards compatible with existing Vulcan
No need to add and remove all sorts of packages that are specific to a UI

luhagel commented 6 years ago

I don't really see why we need to be able to switch frameworks via a setting and register xty frameworks. From my pov, core shouldn't care a single bit about what framework is being used, and just call Components.Whatever. ( Maybe add a check to register some barebones components if none are registered) Then everyone can write their own component packages, extend a framework package or whatnot. Also, since the components are implemented with their respective frameworks in mind, different architecture decisions or props shouldn't be a problem, since you should know which framework/package you are using and pass options accordingly.

SachaG commented 6 years ago

A perfect example is the difference between the bootstrap 3 React library react-bootstrap and the bootstrap 4 library reactstrap

That's a good point, but I feel like it'll probably still be an edge case. For most components, it should be possible to have a common "ancestor"? Also remember that (at least for now) we only need to take care of components in the official Vulcan examples, we don't need to do every single component of every framework right away.

Crafting a new UI framework as in your #3 proposal is basically duplicating the work people have already done with UI frameworks and removes the ability to drop in a purchased css/html theme created using one of the frameworks.

That's why it's last, we might never get to that stage. But see my point above about not needing to build all components; also this would live at the same level as the other UI frameworks so it would not prevent people from using a Bootstrap theme in anyway.

The reason why I think this would be the best solution is that there are plenty of people who don't want to use Bootstrap, especially if it means loading the stylesheet for every element you don't even use. http://sidebar.io is a good example of a project where I technically don't need any component library, but still include Bootstrap's CSS because I don't want to have to style forms myself.

I have not yet worked with Gatsbyjs, yet, but would it be possible to integrate Vulcan with Gatsby?

Yes, but I don't really think that's related to the current discussion. Feel free to create another issue.

However, I don't believe it is an advantage to build for "every" component library (I don't have a better approach) because you will add more work to keep up with all the new developments.

To clarify, I'm not saying I would be the one maintaining all these UI library packages. The idea is just to enable the possibility for devs to easily build/reuse one.

Check it out, then either swap between bootstrap3 or bootstrap4 or semantic in your settings.json

I think this is awesome from a technical point of view, but it really doesn't seem very practical. It's a lot more brittle than simply doing meteor remove vulcan-ui-bootstrap; meteor add vulcan-ui-material.

SachaG commented 6 years ago

Btw, after thinking about it again, let's skip Step 1 for now, because it belongs more to the "give Vulcan its own native UI library" approach (so Step 3). Let's just focus on creating that middle layer of components inside vulcan:core (so that there are not breaking changes for the next version) for now and see where that takes us.

So in other words, our immediate to-do list becomes:

Part 1

[ ] 1. In vulcan:core, move lib/modules/components to /lib/components for consistency's sake
[ ] 2. Create new lib/components/ui directory to hold "middle layer" components.
[ ] 3. Create new lib/components/bootstrap directory to hold bootstrap implementations.
[ ] 4. Sort out current components between those who stay in /component and those who should go in /components/ui
[ ] 5. Create Bootstrap implementations for every /components/ui component.
[ ] 6. Rewrite /components.ui components to use their Bootstrap implementation.

Also, assuming this same approach will work with Formsy (i.e. there will need to be Formsy packages for Material, Semantic, etc.) we can also do:

Part 2

[ ] 1. Bring in Bootstrap components from the vulcan:forms package and follow same pattern.
[ ] 2. Update vulcan:forms to use middle layer components

Part 3

And once this is all done, we can:

[ ] 1. Update vulcan:accounts to use middle layer components

And

Part 4

[ ] 1. Update example-customization to use middle layer components
[ ] 2. Update example-forum to use middle layer components
[ ] 3. Update example-instagram to use middle layer components
[ ] 4. Update example-interfaces to use middle layer components
[ ] 5. Update example-membership to use middle layer components
[ ] 6. Update example-movies to use middle layer components
[ ] 7. Update example-permissions to use middle layer components
[ ] 8. Update example-reactions to use middle layer components
[ ] 9. Update example-simple to use middle layer components

Of course all this will not remove any Bootstrap dependencies just yet but it's a good first step in the right direction.

If you start working on any of this, leave a message here referring the step ("I'm working on 4.5").

ErikDakoda commented 6 years ago

Hey, everybody, I'd like to introduce myself. I'm Erik Dakoda, and I authored https://github.com/ErikDakoda/vulcan-material-ui which implements all of Vulcan core, accounts and forms components using Material UI. I use it in my own project https://beta.etail21.com/.

I'd be happy to contribute to "Make Vulcan UI-library-agnostic" in continuing to maintain vulcan-material-ui and keeping it compatible with the proposed changes.

@SachaG , could you expand further, or perhaps give an example of what the purpose of the "middle layer" would be? So far I have been able to replace Vulcan components that use bootstrap simply with a "replaceComponent". I am not sure what the adding an extra layer would accomplish. Of course, it's important to keep things modular and implementing logic in some comonents and UI in others is helpful. For example, when reskinning vulcan-forms, most of the logic is in Forms.jsx and FromsWrapper.jsx, which I did not have to replace, because they didn't contain form controls or buttons, etc. In fact Forms.jsx did contain a submit button, which I abstracted out into FormsSubmit.jsx and you accepted the pull request.

I do see the value in adding a couple of low level generic components for building UIs, such as Buttons or Modals, but I am not sure if they should call the framework-specific implemetation - I think the framework-specific implenetation should just replace them. Maybe collaborating on a specific example component would reveal the best way to proceed.

MHerszak commented 6 years ago

Here is another use case for replaceComponent. There is only one dependency in Button.jsx if I want to replace AccountsButton. I believe replaceComponent is fully sufficient on a component basis.

SachaG commented 6 years ago

@SachaG , could you expand further, or perhaps give an example of what the purpose of the "middle layer" would be?

I guess it's a bit confusing because forms do already have that middle layer that serves as a "hook" to make components replaceable. But in the Forum code for example there are lots of places where we're doing, say, import { Button } from 'react-bootstrap'. Those are the places where we would like to replace <Button/> with <Components.Button/>.

But you're right, one thing I hadn't really considered or made clear is that this "middle layer" doesn't need to exist "physically". i.e. we don't need a Button.jsx file somewhere, it's enough to call <Components.Button/> and then let BootstrapButton.jsx, MaterialButton.jsx, etc. each register the Button component.

So basically we're talking about extending the patterns applied in vulcan:forms to all other components that currently depend on Bootstrap.

ErikDakoda commented 6 years ago

Sounds good to me!

MHerszak commented 6 years ago

This is how I implement reactstrap components.

import StrapComponents from 'reactstrap';

import { registerComponent } from 'meteor/vulcan:core';

Object.keys(StrapComponents).forEach(component => {
    registerComponent(component, StrapComponents[component]);
});

Had no problems so far.

MHerszak commented 6 years ago

First problem FormsyControls. They have the same keys.

const BLACKLIST = ['FormGroup'];
Object.keys(StrapComponents).forEach(component => {
    if (!BLACKLIST.includes(component)) {
        registerComponent(component, StrapComponents[component]);
    }
});

Meaning, formsy components need a Vulcan prefix.

ErikDakoda commented 6 years ago

@MHerszak I don't think you should be registering every component that comes with reactstrap. There are likely to be naming conflicts. I would suggest that you use the low-level components in reactstrap with simple import statements and only use Vulcan's for higher level components that replace Vulcan components or that you write for your app.

However, the following should probably work:

registerComponent("ReactStrap" + component, StrapComponents[component]);

MHerszak commented 6 years ago

@ErikDakoda you are right. However, how would want to incorporate with some of the core elements? When most elements have "pure" names such as Button? And Button is less cluttered. Am I thinking too complicated? Would you add ReactStrap as a prefix when you work on your custom project? What happens to all your existing Button, FormGroup elements? Wouldn't that be overhead when your value store has Button but will never be used instead it will use ReactStrapButton? I might think too complicated here.

ErikDakoda commented 6 years ago

I would not register reactstrap's components with Vulcan's registerComponent and I would continue to use a reactstrap Button, for example as <Button/>, not <Components.Button/>. I think you are making it more complicated than necessary

SachaG commented 6 years ago

Leaving this here as a possible option for forms: https://boilerform.design/

ghost commented 6 years ago

Unfortunately, Semantic-UI icons don't work with Vulcan. It is the same problem that happens if you install them on Meteor without following the Usage chapter. Fortunately, on Meteor you can make the changes, but in Vulcan you cannot remove those packages and, also, there are no instruction on how to integrate them. I believe Semantic-UI is a powerful library, being english-centered, intuitive and visually better than other CSS libraries. Plus it has react integration

SachaG commented 6 years ago

@gabimoncha see https://github.com/VulcanJS/Vulcan/issues/1935#issuecomment-376530313

SachaG commented 6 years ago

I've started working on this: https://github.com/VulcanJS/Vulcan/tree/ui2

I've already removed all Bootstrap dependency from the core, now I just need to update the starter packages.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

VulcanJS / Vulcan