-
Notifications
You must be signed in to change notification settings - Fork 78
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
DOC-458 OmniboxResultList documentation revision #345
DOC-458 OmniboxResultList documentation revision #345
Conversation
fbeaudoincoveo
commented
Feb 13, 2017
@@ -108,8 +129,8 @@ export class OmniboxResultList extends ResultList implements IComponentBindings | |||
} | |||
|
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.
Would you like me to expose the renderResults public method in the doc?
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.
Yes, it would be useful.
* This component is exactly like a normal ResultList Component, except that it will render itself inside the Omnibox component. | ||
* This will provide a kind of search as you type functionnality, allowing you to easily render complex Result Templates inside the Omnibox component. | ||
* The OmniboxResultList component acts exactly like a normal {@link ResultList} component, except that it renders itself | ||
* inside the {@link Omnibox} component. | ||
* |
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 considered mentioning that this component actually inherits from the ResultList component... However, since most of the ResultList options don't really seem to apply to the OmniboxResultList, I finally decided against it. What do you think?
@@ -108,8 +129,8 @@ export class OmniboxResultList extends ResultList implements IComponentBindings | |||
} | |||
|
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.
Yes, it would be useful.
- Mentioned that OmniboxResultList extends ResultList - Exposed RenderResults method - Several other minor fixes in public documentation
* appends to the result container, this method triggers a `newResultDisplayed` event. Once all items have been | ||
* appended to the result container, the method triggers a `newResultsDisplayed` event. | ||
* @param resultsElement The array of `HTMLElement` to render. | ||
* @param append |
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.
The append
parameter is not used in the method, do we need to keep it?
- Fixed typo in public doc