Skip to content

Drawer#90

Merged
michaelmang merged 5 commits intomasterfrom
drawer
May 18, 2020
Merged

Drawer#90
michaelmang merged 5 commits intomasterfrom
drawer

Conversation

@michaelmang
Copy link
Contributor

@michaelmang michaelmang requested a review from nicko-winner May 15, 2020 20:05
@netlify
Copy link

netlify bot commented May 15, 2020

Deploy preview for helix-react ready!

Built with commit 0e06577

https://deploy-preview-90--helix-react.netlify.app

@michaelmang michaelmang requested a review from 100stacks May 15, 2020 20:05
@michaelmang
Copy link
Contributor Author

@nicko-winner ready for re-review

open={wcBool(open)}
{...rest}
>
<hx-modal class={classNames(className, SIZES[size])} ref={hxRef} open={wcBool(open)} {...rest}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this prettier formatting? For me it's not as readable, though I defer to you @HelixDesignSystem/helix-react-core

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is based on the length of the line. It breaks when it exceeds 100 characters: https://github.com/HelixDesignSystem/helix-react/blob/master/prettier.config.js#L4

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok thanks @michaelmang. 👍

I wonder if we can change that config...though that's very minor.

@michaelmang
Copy link
Contributor Author

@100stacks all feedback should be addressed

Copy link
Member

@100stacks 100stacks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dev LGTM

@michaelmang michaelmang merged commit e779a9f into master May 18, 2020
@michaelmang michaelmang deleted the drawer branch May 18, 2020 17:08
@100stacks 100stacks added this to the v1.0.0-rc.0 milestone Jul 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants