Rmwc: MDC 30

Created on 6 Feb 2018  Â·  24Comments  Â·  Source: jamesmfriedman/rmwc

  • [x] Rename SimpleMenu to Menu and all relevant classes
  • [x] Create an RMWC simple menu that can automatically handle the open state
  • [x] It seems like dark themes have been removed altogether? Validate this and adjust accordingly.
  • [x] Add a chip component
  • [x] remove cssOnly select
  • [x] add box version of Select and validate non-box version works appropriately
  • [x] mdc-text-field__bottom-line -> mdc-line-ripple
  • [x] mdc-checkbox__checkmark__path -> mdc-checkbox__checkmark-path
  • [x] Cards layouts have been completely gutted, respond accordingly
  • [x] addition of mdc-button__icon
  • [x] return of the mini fab... mdc-fab--mini
  • [ ] match minor version to MDC for next release.
enhancement help wanted

All 24 comments

What's the reason for removing all of the "css-only" components? Seems like something lot's of users actually need. Especially for a select component e.g. on mobile...

Regarding Textfield mdc-text-field__bottom-line. I think it is mdc-line-ripple now and it will be a new component in rmwc?

Also, how can I add breaking changes into the changelog? Regarding the change of SimpleMenu -> Menu?

Thanks for jumping on this one @j-o-d-o!

  • I think the reason they are removing the css-only components is because they can't guarantee styling or functionality across browsers. In the instance of select, it makes a lot of sense, but the cssOny one never matched up exactly with the JS one. Whats your thoughts on supporting a cssOnly select directly in RMWC? It would be the first attempt at preventing a breaking change but might also involve doing some stying which RMWC doesn't currently include.

  • You are correct on mdc-line-ripple. Apparently they called it bottom line first and then changed the name

  • for the changelog, follow the patterns here. For breaking you can simply type breaking(Select): my description.... I know it's annoying, but if you can amend that specific commit title then the changelog will be generated appropriately.

  • Lets keep that mdc30 branch open and keep working off of it until we get all of the features in there.

  • Maybe we can convince the people over at material-components-web to have a blog related to why they are doing the things they are? It would really help with dramatic changes like this. I don't know how to get them to do that though other than opening an issue that looks like I'm just complaining. Either way, I asked them to keep the docs updated politely and that hasn't happened at all :(.

  • I would opt against specific RMWC cssOnly components, mainly because of having to bring in styling to RMWC. Let's keep it simple ; ).
  • I'd rather prefer to know why major breaking changes are happening. Not sure though how to properly communicate that ... I think it would even be enough if they have a proper statement on the PRs instead of a message like: "Remove css only version of select." ... I mean, thanks a lot, I can see that information in the PR title already...
  • Changed the commit messages, had to rebase and force push, so I hope it doesn't mess up anything for others working on the branch.

Ya they seem to be chugging along to their own song. Seems like they are working on a Material 2.0 atm and material-web-components is moving in that direction. Think they will announce at I/O this year. They do have a pivotal tracker as well.

So when the rmwc user wants to init the menu with an open state, he/she has to use the startOpen property. But of course the open prop must be true also, otherwise it is open but will close onUpdate(). I'd say that is common sense? It would be possible to ignore the open property until it first changes, but I think that would just kind of correct an error in the users react code?

Also I am not sure this is a bug, or is the menu supposed to be to the right of the button when using mdc-menu--open? https://codepen.io/j-o-d-o/pen/zppEdL

It doesn't make much sense to me to expose a startOpen prop. Open should mean open in case of React. We should just catch it and adjust accordingly.

The SimpleMenu component I mentioned in the list is just an idea to have a component that automatically manages the open state because in my experience, 99% of the time you just need the menu to open when you click a button, and close when something is clicked or clicked off of.

Believe it or not, I actually have another UI library for work that wrap this one (mostly to add additional items, and style for our apps). This is the wrapper I wrote for it:

/**
 * A Simplified menu component that allows you to pass a handle element and will automatically control the open state and add a MenuAnchor
 */
export class SimpleMenu extends React.Component<
  SimpleMenuPropsT,
  SimpleMenuStateT
> {
  static displayName = 'SimpleMenu';

  componentWillMount() {
    this.syncWithOpenProp(this.props.open);
  }

  componentWillReceiveProps(nextProps) {
    this.syncWithOpenProp(nextProps.open);
  }

  state = {
    open: false
  };

  syncWithOpenProp(open) {
    if (open !== undefined && this.state.open !== open) {
      this.setState({ open });
    }
  }

  render() {
    const {
      handle,
      onClose,
      children,
      rootProps = {},
      open,
      ...rest
    } = this.props;
    const wrappedHandle = React.cloneElement(handle, {
      ...handle.props,
      onClick: evt => {
        this.setState({ open: true });
        if (handle.props.onClick) {
          handle.props.onClick(evt);
        }
      }
    });

    const wrappedOnClose = evt => {
      this.setState({ open: false });
      if (onClose) {
        onClose(evt);
      }
    };
    return (
      <MenuAnchor {...rootProps}>
        <Menu onClose={wrappedOnClose} open={this.state.open} {...rest}>
          {children}
        </Menu>
        {wrappedHandle}
      </MenuAnchor>
    );
  }
}

Usage

<SimpleMenu handle={<Button>Open Me<Button>}>
  <MenuItem />
  ...
</SimpleMenu>

Btw, thanks for your help on this @j-o-d-o and @izakfilmalter . Haven't had time to get to it yet this week and it's awesome to see progress being made.

@jamesmfriedman yeah that actually makes more sense, I will adjust that!

The SimpleMenu looks like a convenient addition to the library.

I used the api.quickOpen to open the menu on mounting without animation if the initial state of open=true.

Perfect, thanks @j-o-d-o. I'm going to jump in and write some code for this tonight and tomorrow. Thanks for getting it off to a get start.

@j-o-d-o I'm going to work on chips, button icon, dark theme removal and fab mini

Just need to take care of the cards and we'll be good to go.

Also @lynnjepsen dropped an issue with #141. I'm curious at looking into doing a direct foundation implementation to see how it matches up. Probably out of scope for this particular release, but wanted to look into it and see if it was worth taking a simpler component and integrating with it.

Lastly, whats your thoughts on matching the release numbers up with MCW? We can't drop below the major version, but maybe make it 1.30.0 to kind of match MCW 0.30.0?

Pretty much wrapped up @j-o-d-o and @izakfilmalter. Please review the changes. I have one small issue I want to inspect (text field labels seems misaligned) but other than that I'm good to go.

I'm really rethinking the cssOnly select removal.

As far as RMWC goes, why not provide a wrapper? There is already a non-standard declarative way to handle selects in a data-driven way. It seems trivial to render a standard select element with options and just tell people they are required to style it on their own.

I know this would make @izakfilmalter life a lot easier. Otherwise you always have to handle both your self which seems pretty terrible.

And when MDC decides to add it back in (which I'm betting they do some day), it's an easy upgrade.

I wouldn't mind if it was a package and this became like a mono repo. Long
term it's tough. You have to track visual bugs in a bunch of browsers, yet
I am gonna have to make one my self to update to this release.

On Mon, Feb 12, 2018, 2:12 PM James Friedman notifications@github.com
wrote:

I'm really rethinking the cssOnly select removal.

As far as RMWC goes, why not provide a wrapper? There is already a
non-standard declarative way to handle selects in a data-driven way. It
seems trivial to render a standard select element with options and just
tell people they are required to style it on their own.

I know this would make @izakfilmalter https://github.com/izakfilmalter
life a lot easier. Otherwise you always have to handle both your self which
seems pretty terrible.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/jamesmfriedman/rmwc/issues/139#issuecomment-365030034,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFmimglJUAnAN4CF8DFbzzmToVlMxt88ks5tUI0ggaJpZM4R6c-g
.

Can you elaborate? Whats a mono-repo?

Kinda like create-react-app. I think they use https://github.com/lerna/lerna
to have many npm packages in the same repo.

On Mon, Feb 12, 2018, 3:16 PM James Friedman notifications@github.com
wrote:

Can you elaborate? Whats a mono-repo?

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/jamesmfriedman/rmwc/issues/139#issuecomment-365048309,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFmimrkYVNUpdG4Dau6F0q3t4yUEbAPHks5tUJwFgaJpZM4R6c-g
.

Awww. Yes, I would like that a lot, especially because I want to support community packages and allow for people to only install what they need.

Until then, do you think we should the Select handle cssOnly in the main code base?

I think it's out of scope.

On Mon, Feb 12, 2018, 3:45 PM James Friedman notifications@github.com
wrote:

Awww. Yes, I would like that a lot, especially because I want to support
community packages and allow for people to only install what they need.

Until then, do you think we should the Select handle cssOnly in the main
code base?

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/jamesmfriedman/rmwc/issues/139#issuecomment-365056226,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFmimkhwsnH6Xbbgct1Z0RIpgoDIlszpks5tUKMGgaJpZM4R6c-g
.

Didn't end up matching the minor version, but the rest is done and deployed. Good job guys! 🥇

Really nice but open=true doesn't seem to work when we have styles that override the styles of the MenuItem. I mean it still does the animation for the text and background color of the MenuItems.

@binarybaba can you elaborate please? This is a really old issue, and the library has changed quite a bit since. If you’re experiencing a problem can you open a new issue?

Was this page helpful?
0 / 5 - 0 ratings