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

Make target optional for Drop typescript type #3100

Merged
merged 1 commit into from May 11, 2019

Conversation

Projects
None yet
2 participants
@hugomn
Copy link
Contributor

commented May 11, 2019

What does this PR do?

Drop target should not be a required prop. When using Menu, passing dropProps fail typescript validation when you don't provide target, as opposed to the documentation.

Where should the reviewer start?

N/A

What testing has been done on this PR?

Yes.

How should this be manually tested?

Use Menu with a typescript setup, and don't provide target for dropProps.

Any background context you want to provide?

No.

What are the relevant issues?

N/A

Screenshots (if appropriate)

image

Do the grommet docs need to be updated?

No.

Should this PR be mentioned in the release notes?

I don't think so.

Is this change backwards compatible or is it a breaking change?

Yes.

@ghost ghost added the in progress label May 11, 2019

@ShimiSun

This comment has been minimized.

Copy link
Member

commented May 11, 2019

Congrats and Thanks for your first contribution to Grommet 🎉 see you around!

@ShimiSun ShimiSun merged commit 5950ebb into grommet:master May 11, 2019

7 of 10 checks passed

Header rules - sad-tereshkova-173d07 No header rules processed
Details
Pages changed - sad-tereshkova-173d07 All files already uploaded
Details
Redirect rules - sad-tereshkova-173d07 No redirect rules processed
Details
Mixed content - sad-tereshkova-173d07 No mixed content detected
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: bundlesize Your tests passed on CircleCI!
Details
ci/circleci: checkout Your tests passed on CircleCI!
Details
ci/circleci: jest Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
netlify/sad-tereshkova-173d07/deploy-preview Deploy preview ready!
Details

@ghost ghost removed the in progress label May 11, 2019

@hugomn

This comment has been minimized.

Copy link
Contributor Author

commented May 12, 2019

Thanks, @ShimiSun! 😍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.