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

Will/popup #33

Merged
merged 14 commits into from
Jun 13, 2020
Merged

Will/popup #33

merged 14 commits into from
Jun 13, 2020

Conversation

WillHHH
Copy link
Collaborator

@WillHHH WillHHH commented Jun 1, 2020

No description provided.

src/components/Popup/index.scss Outdated Show resolved Hide resolved
src/components/Popup/index.scss Outdated Show resolved Hide resolved
src/components/Popup/index.scss Outdated Show resolved Hide resolved
src/components/Popup/index.stories.tsx Outdated Show resolved Hide resolved
src/components/Popup/index.stories.tsx Outdated Show resolved Hide resolved
src/components/Popup/index.tsx Outdated Show resolved Hide resolved
src/components/Popup/index.tsx Outdated Show resolved Hide resolved
src/utils/classNames.ts Outdated Show resolved Hide resolved
@WillHHH WillHHH changed the title Will/popup-wip Will/popup Jun 2, 2020
@z695245771
Copy link
Collaborator

image
image
Looks bad on windows dude

trista8130
trista8130 previously approved these changes Jun 5, 2020
Copy link
Collaborator

@trista8130 trista8130 left a comment

Choose a reason for hiding this comment

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

LGTM

src/components/Popup/types.ts Outdated Show resolved Hide resolved
Object.assign(popupProps, {
style: {
...popupProps.style,
borderTopLeftRadius:
Copy link
Owner

Choose a reason for hiding this comment

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

this logic needs to be simplified, also you need to use class name here

src/components/Popup/index.tsx Outdated Show resolved Hide resolved
trista8130
trista8130 previously approved these changes Jun 8, 2020
Copy link
Collaborator

@ssysm ssysm left a comment

Choose a reason for hiding this comment

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

.includes vs .indexOf

@WillHHH WillHHH merged commit 24dfe8b into master Jun 13, 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.

None yet

6 participants