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

fix(HorizontalNavMenuItem): Fix onItemClick PropType. #2984

Merged

Conversation

@martinpovolny
Copy link
Contributor

martinpovolny commented Sep 23, 2019

HorizontalNavMenuItem 's onItemClick should be a function not an object.

It's getting passed as onClick to <a href ...> so it must be a bug.

Issue: #2994

Ping @rvsia, @karelhala

HorizontalNavMenuItem 's onItemClick should be a function not an object.
Copy link
Contributor

karelhala left a comment

Good catch!

@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Sep 23, 2019

PatternFly-React preview: https://patternfly-react-pr-2984.surge.sh

@rvsia
rvsia approved these changes Sep 23, 2019
martinpovolny added a commit to martinpovolny/react-ui-components that referenced this pull request Sep 23, 2019
Copy link
Contributor

tlabaj left a comment

Can you please link associated issue. Thanks!

@martinpovolny

This comment has been minimized.

Copy link
Contributor Author

martinpovolny commented Sep 23, 2019

Can you please link associated issue. Thanks!

There's none.

Do we need 6 people and an issue to merge a trivial 1 line fix to Patternfly?

@dlabaj

This comment has been minimized.

Copy link
Contributor

dlabaj commented Sep 24, 2019

@martinpovolny We do need an issue for all bugs (https://github.com/patternfly/patternfly-react/blob/master/CONTRIBUTING.md#creating-issues-for-bugs). We only need 2 approvals to merge a PR.

@martinpovolny

This comment has been minimized.

Copy link
Contributor Author

martinpovolny commented Sep 24, 2019

@dlabaj : Thank you for the information. Updating counter to 7 ;-)

I have created the issue as you required: #2994

However I cannot follow the steps you pointed me to 100%. I cannot assing the label because I do not have the permisions.

Regards.

@karelhala karelhala added the bug 🐛 label Sep 24, 2019
@dlabaj
dlabaj approved these changes Sep 24, 2019
@dlabaj dlabaj dismissed tlabaj’s stale review Sep 24, 2019

change was made

@dlabaj dlabaj merged commit 90e1792 into patternfly:master Sep 24, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Sep 24, 2019

Your changes have been released in:

  • patternfly-react-extensions@2.20.6
  • patternfly-react@2.39.1
  • @patternfly/react-console@1.12.10

Thanks for your contribution! 🎉

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