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

DOC-458 FacetSlider documentation revision #359

Merged
merged 3 commits into from
Feb 15, 2017
Merged

DOC-458 FacetSlider documentation revision #359

merged 3 commits into from
Feb 15, 2017

Conversation

fbeaudoincoveo
Copy link
Contributor

Deploy

*/
responsiveBreakpoint: ComponentOptions.buildNumberOption({ defaultValue: 800 }),
responsiveBreakpoint: ComponentOptions.buildNumberOption({ defaultValue: 800, min: 0 }),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a minimum value here; please make sure it's OK.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.185% when pulling 5109e1a on fbeaudoincoveo:FacetSliderDocRevision into c67b6ba on coveo:master.

* Specifies whether the field for which you are requesting a range is a date field.<br/>
* This allows the facet to correctly build the outgoing group by request, as well as render it correctly.<br/>
* Specifies whether the field for which you are requesting a range is a date field. This allows the FacetSlider to
* correctly build the outgoing {@link IGroupByRequest} and render itself properly.
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you could do [GroupByRequest]{@link IGroupByRequest} so that it shows up as "GroupByRequest".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea, I'll eventually do this in other places in the doc, too!

* Specifies the number of steps to split the slider into.
*
* For example, if your range is [ 0 , 100 ] and you specify 10 steps, then the end user can move the slider only to
* the values [ 0, 10, 20. 30 ... , 100 ].
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a period instead of a comma after the 20

* Specifies a function that will generate the steps for the slider. The function receives the slider boundaries and must return an array of number (the steps).
* Specifies a function to generate the steps for the FacetSlider (see {@link FacetSlider.options.steps}. This
* function receives the FacetSlider boundaries (see {@link FacetSlider.options.start} and
* {@link FacetSlider.options.end}) and must return an array of number (the steps).
Copy link
Contributor

Choose a reason for hiding this comment

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

numbers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since "number" is the type, I hesitate to use the plural form. Perhaps I could instead emphasize it, like "array of number". What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I discussed it with François Dallaire and he agrees with your idea of using plural when talking about multiple items of the same type, so I'm going to do this after all :)

*/
responsiveBreakpoint: ComponentOptions.buildNumberOption({ defaultValue: 800 }),
responsiveBreakpoint: ComponentOptions.buildNumberOption({ defaultValue: 800, min: 0 }),
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.185% when pulling 726ae4c on fbeaudoincoveo:FacetSliderDocRevision into c67b6ba on coveo:master.

* Coveo.init(document.querySelector('#search'), {
* FacetSlider: {
* field: "@size",
* field: "@syssize",
Copy link
Member

Choose a reason for hiding this comment

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

Never use @sys fields in documentation. @sys fields are deprecated (still works, but deprecated)

* $('#search').coveo('init', {
* FacetSlider: {
* field: "@size",
* field: "@syssize",
Copy link
Member

Choose a reason for hiding this comment

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

same

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.185% when pulling ddb8770 on fbeaudoincoveo:FacetSliderDocRevision into c67b6ba on coveo:master.

@olamothe olamothe merged commit 351ec41 into coveo:master Feb 15, 2017
@fbeaudoincoveo fbeaudoincoveo deleted the FacetSliderDocRevision branch February 16, 2017 13:52
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.

4 participants