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

Help wanted: update starter packages for new ui-bootstrap package #50

Open
SachaG opened this issue Apr 24, 2018 · 7 comments
Open

Help wanted: update starter packages for new ui-bootstrap package #50

SachaG opened this issue Apr 24, 2018 · 7 comments

Comments

@SachaG
Copy link
Contributor

SachaG commented Apr 24, 2018

Goal: remove all dependencies on react-bootstrap from Starter repo

The example packages currently rely a lot on Bootstrap in the form of dependencies on react-bootstrap. But we can now get rid of this dependency by replacing all Bootstrap components with their equivalent Vulcan component.

In other words, <Button/> imported from react-bootstrap becomes <Components.Button/>; which in turn can call a Bootstrap, Material, Semantic, etc. component according to which UI package is currently active.

You can see all currently available components here.

TODO:

  • Search for dependencies on react-bootstrap throughout the Starter repo.
  • Once you find one, replace it with the equivalent component.
  • Submit PR!
@SachaG
Copy link
Contributor Author

SachaG commented Apr 24, 2018

Since the dropdown component is probably the most complex to port, here is an example from zenshome.jp:

const UsersMenuItem = ({ showItem, name, isAdmin = false }) => showItem && (
  <div>
    <FormattedMessage id={`nav.${name}`} />
    {isAdmin && (
      <span className="users-menu-admin-marker">
        <FormattedMessage id="nav.admin_marker" />
      </span>
    )}
  </div>
);
const menuItems = [
  {
    name: 'account',
    to: '/account',
  },
  {
    name: 'users',
    to: '/admin/users',
    isAdmin: true,
  },
  {
    name: 'bookings',
    to: '/admin/bookings',
    isAdmin: true,
  },
  {
    name: 'rooms',
    to: '/admin/rooms',
    isAdmin: true,
  },
  {
    name: 'reviews',
    to: '/admin/reviews',
    isAdmin: true,
  },
  {
    name: 'neighborhoods',
    to: '/admin/neighborhoods',
    isAdmin: true,
  },
];
<Components.Dropdown
      variant="default"
      id="user-dropdown"
      trigger={
        <div className="dropdown-toggle-inner">
          <Components.Avatar size="small" user={currentUser} link={false} />
          <div className="users-menu-name">
            {Users.getDisplayName(currentUser)}
          </div>
        </div>
      }
      pullRight
      menuItems={[
        // menu items
        ...menuItems.map((item, index) => ({
          to: item.to,
          component: <UsersMenuItem {...item} key={index} index={index} showItem={showItem(item, currentUser)}/>,
        })),
        // log out
        {
          component: <FormattedMessage id="users.log_out" />,
          itemProps: {
            onClick: () => Meteor.logout(() => client.resetStore()),
          },
        },
      ]}
    />

@SachaG
Copy link
Contributor Author

SachaG commented Apr 24, 2018

Oh and feel free to also submit PRs back to the core repo to improve the ui-bootstrap package.

@SachaG
Copy link
Contributor Author

SachaG commented Apr 27, 2018

I'll probably work on this myself since there are no takers. Also a good chance to review/clean up the Forum code generally.

@kabalin
Copy link
Contributor

kabalin commented Apr 27, 2018

I was going to take it. In fact I have done it partly.

kabalin added a commit to kabalin/Vulcan-Starter that referenced this issue Apr 27, 2018
Moving away of direct react-bootstrap dependency (issue VulcanJS#50).
kabalin added a commit to kabalin/Vulcan-Starter that referenced this issue Apr 27, 2018
Moving away of direct react-bootstrap dependency (issue VulcanJS#50).
@SachaG
Copy link
Contributor Author

SachaG commented Apr 27, 2018

Oh in that case please do! I haven't actually started on this :)

kabalin added a commit to kabalin/Vulcan-Starter that referenced this issue Apr 27, 2018
Moving away of direct react-bootstrap dependency (issue VulcanJS#50).

Patch dos not include Dropdown and Modal refactoring yet.
@kabalin
Copy link
Contributor

kabalin commented Apr 27, 2018

@SachaG yep, will do in that case, it is a good excercise for me :)

kabalin added a commit to kabalin/Vulcan-Starter that referenced this issue May 1, 2018
Moving away of direct react-bootstrap dependency (issue VulcanJS#50).
kabalin added a commit to kabalin/Vulcan-Starter that referenced this issue May 1, 2018
Moving away of direct react-bootstrap dependency (issue VulcanJS#50).
kabalin added a commit to kabalin/Vulcan-Starter that referenced this issue May 1, 2018
Moving away of direct react-bootstrap dependency (issue VulcanJS#50).

Patch dos not include Dropdown and Modal refactoring yet.
kabalin added a commit to kabalin/Vulcan-Starter that referenced this issue May 1, 2018
Moving away of direct react-bootstrap dependency (issue VulcanJS#50).
@JstnEdr
Copy link

JstnEdr commented May 11, 2018

There was an error coming up for many of the starter packages that were missing an import in the package.json file. I've added 'vulcan:ui-bootstrap@1.10.0' to all the starter package files and will submit a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants