Skip to content

docs: add code samples APIC-348 #202

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

Merged
merged 3 commits into from
Mar 2, 2022
Merged

Conversation

millotp
Copy link
Collaborator

@millotp millotp commented Mar 1, 2022

🧭 What and Why

🎟 JIRA Ticket: APIC-348

I suck at writing doc.

There is not much code sample in this, I need to add more examples

🧪 Test

yarn website

@millotp millotp self-assigned this Mar 1, 2022
@netlify
Copy link

netlify bot commented Mar 1, 2022

✔️ Deploy Preview for api-clients-automation ready!

🔨 Explore the source changes: 5542c49

🔍 Inspect the deploy log: https://app.netlify.com/sites/api-clients-automation/deploys/621f7a6e674e820007880704

😎 Browse the preview: https://deploy-preview-202--api-clients-automation.netlify.app

@millotp millotp changed the title feat(doc): add code samples APIC-348 feat(docs): add code samples APIC-348 Mar 1, 2022
@millotp millotp requested review from a team, eunjae-lee, damcou and shortcuts and removed request for a team March 1, 2022 15:42
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

nice :D small comment since I've just made a change on introduction

Add the JavaScript to your `<head>`

```html
<script src="https://cdn.jsdelivr.net/npm/@experimental-api-clients-automation/[email protected]/dist/algoliasearch.umd.browser.min.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<script src="https://cdn.jsdelivr.net/npm/@experimental-api-clients-automation/[email protected]/dist/algoliasearch.umd.browser.min.js"></script>
<script src="https://cdn.jsdelivr.net/npm/@experimental-api-clients-automation/[email protected]/dist/algoliasearch.umd.browser.js"></script>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Weird that the min is bigger than the regular file

Copy link
Member

Choose a reason for hiding this comment

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

We don't provide a min for the generate clients, the umd is the min one

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

jsdelivr gave me the min url by default, but it generated the others too.

Copy link
Member

Choose a reason for hiding this comment

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

Hm weird, I'll check but since we only provide one umd file there's no min/normal

@shortcuts
Copy link
Member

I suck at writing doc.

It's fine!! and docusaurus makes it funnier too!

There is not much code sample in this, I need to add more examples

We could have an example for an other client like personalization, but otherwise I think it's enough to give a starting point.

const client = algoliasearch(appId, apiKey);

// And access other clients from here
const analyticsClient = algoliaClient.initAnalytics(analyticsAppId, analyticsApiKey);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a place where we can see all the list of initXYZ methods? (It shouldn't be here, because it can be too much, but it would be nice to link to the page)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so, and actually algoliasearch only provides analytics and personalization, (which is a weird choice IMO), should we provide more?

Copy link
Contributor

Choose a reason for hiding this comment

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

@shortcuts ah okay, I didn't have the full context. So if algoliasearch only provides analytics and personalization, then how do people use other clients with algoliasearch? Is there a way? (And can you explain to me the decision? I might've missed something.)

Copy link
Member

@shortcuts shortcuts Mar 1, 2022

Choose a reason for hiding this comment

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

And can you explain to me the decision?

For now I only tried to reproduce the current usage, which only provide those two

algoliasearch/lite only provide 5 search methods to be the lightest possible
algolisearch default to the search client + allow usage of analytics and personalization

IMO we should provide all of the clients under algoliasearch and avoid touching the lite version

Copy link
Member

Choose a reason for hiding this comment

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

then how do people use other clients with algoliasearch?

They can use the package directly, e.g. @algolia/client-abtesting

Copy link
Contributor

Choose a reason for hiding this comment

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

I see! Then I guess it's okay for now to just explain here that we provide only initAnalytics and initPersonaliazation.

When we decide on changes on what to expose and what not, then we can revisit this doc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it normal that the client is named algoliaClient here ?

Copy link
Member

Choose a reason for hiding this comment

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

Good catch, it should be client


console.log('Search result', res.hits);
```

Copy link
Contributor

Choose a reason for hiding this comment

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

Are we planning a live example like CodeSandbox?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

could be cool ! don't we already have that for the actual client ?

@millotp millotp changed the title feat(docs): add code samples APIC-348 docs: add code samples APIC-348 Mar 1, 2022
Copy link
Contributor

@damcou damcou left a comment

Choose a reason for hiding this comment

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

LGTM (Just had a small question on an existing discussion above)

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

Good for me!

@millotp millotp requested a review from shortcuts March 2, 2022 14:08
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

GGGGGG

@millotp millotp merged commit e6437a8 into feat/APIC-260/api-doc Mar 2, 2022
@millotp millotp deleted the feat/code-sample branch March 2, 2022 14:10
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