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-593 firstLoadingAnimation: fixed inconsistencies with Developers doc #379

Merged
merged 11 commits into from
Mar 7, 2017
Merged

DOC-593 firstLoadingAnimation: fixed inconsistencies with Developers doc #379

merged 11 commits into from
Mar 7, 2017

Conversation

fbeaudoincoveo
Copy link
Contributor

  • Also made some minor cosmetic changes to the SearchInterface documentation.

Deploy

- Also made some minor cosmetic changes to the SearchInterface documentation.
- Mentioned that you can add "-selector" to the infiniteScrollContainer option to specify a CSS selector (since this option is built with ComponentOptions.buildChildHtmlElementOption)
@@ -209,6 +209,9 @@ export class ResultList extends Component {
* If {@link ResultList.options.enableInfiniteScroll} is `true`, specifies the element that triggers the fetching of
* additional results when the end user scrolls down to its bottom.
*
* You can change the container by specifying its selector (e.g.,
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 this because this option is built with ComponentOptions.buildChildHtmlElementOption. Please make sure what I wrote is actually right :)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.59% when pulling 9a59980 on fbeaudoincoveo:DOC-593 into c6592af on coveo:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.59% when pulling 9a59980 on fbeaudoincoveo:DOC-593 into c6592af on coveo:master.

@@ -138,7 +138,7 @@ export class SearchInterface extends RootComponent implements IComponentBindings
* > {@link Facet.options.dropdownHeaderLabel} and {@link Tab.options.dropdownHeaderLabel}).
* >
* > Furthermore, it is possible to specify the pixel threshold at which Facet components will go in responsive
* > mode (see {@link Facet.options.responsiveBreakpoint}.
* > mode (see {@link Facet.options.responsiveBreakpoint}).
Copy link
Member

Choose a reason for hiding this comment

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

We modified that, but the doc has not been adjusted.

The breakpoint is no longer specified per component. It's a global setting, on the search interface component (SearchInterface.responsiveComponents).

So this should be modified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Should I also deprecate Facet.options.responsiveBreakpoint then?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

- Some other slight documentation adjustments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 76.569% when pulling dbe147e on fbeaudoincoveo:DOC-593 into c6592af on coveo:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 76.569% when pulling 8d39e21 on fbeaudoincoveo:DOC-593 into c6592af on coveo:master.

*/
responsiveBreakpoint: ComponentOptions.buildNumberOption({ defaultValue: 800 }),
responsiveBreakpoint: ComponentOptions.buildNumberOption({ defaultValue: 800, deprecated: 'This option is exposed for legacy reasons, and the recommendation is to not use this option.' }),
Copy link
Contributor

Choose a reason for hiding this comment

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

It does not work anymore so you should probably recommend to use SearchInterface.responsiveComponents here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I did exactly this in the previous commit :)

Copy link
Member

Choose a reason for hiding this comment

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

I would also go as far as completely removing the JSDOC for deprecated doc. It serves pretty much no purposes, and only pollutes the doc website with useless info.

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 just realized I understood what you said the other way around ;)

I'll make a new commit to fix this.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.569% when pulling 67d1aec on fbeaudoincoveo:DOC-593 into ce1efff on coveo:master.

*/
responsiveBreakpoint: ComponentOptions.buildNumberOption({ defaultValue: 800 }),
responsiveBreakpoint: ComponentOptions.buildNumberOption({ defaultValue: 800, deprecated: 'This option is exposed for legacy reasons, and the recommendation is to not use this option.' }),
Copy link
Member

Choose a reason for hiding this comment

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

I would also go as far as completely removing the JSDOC for deprecated doc. It serves pretty much no purposes, and only pollutes the doc website with useless info.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.569% when pulling ef6aef2 on fbeaudoincoveo:DOC-593 into 7025331 on coveo:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.569% when pulling ef6aef2 on fbeaudoincoveo:DOC-593 into 7025331 on coveo:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.569% when pulling 3d29be0 on fbeaudoincoveo:DOC-593 into 7025331 on coveo:master.

@olamothe olamothe merged commit 8246c97 into coveo:master Mar 7, 2017
@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.569% when pulling c397bcb on fbeaudoincoveo:DOC-593 into 715ae9b on coveo:master.

@fbeaudoincoveo fbeaudoincoveo deleted the DOC-593 branch March 8, 2017 19:34
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