-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[UI Framework] Improve navigability of the docs #11666
[UI Framework] Improve navigability of the docs #11666
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks for doing this! Great addition. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this. It's going to be very helpful!
I left a minor comment on the code. Even without that change, this PR LGTM.
this.onSearchChange = this.onSearchChange.bind(this); | ||
} | ||
|
||
onSearchChange(event) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if you redefine this as onSearchChange = (event) => {
you can get rid of the this
binding in the constructor. Personally I've found this style more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried it out and got a syntax error. I don't think it's valid ES6 class syntax? 🍂
* Add search to navigation. * Add pagination buttons to UI Framework header.
* Add search to navigation. * Add pagination buttons to UI Framework header.
This adds search and pagination to the UI Framework. Addresses #11574.