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

[New sample] Open Chrome API reference page (omnibox, alarms, messaging sample) #848

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

AmySteam
Copy link
Contributor

User Story

As a web developer, I want to open extension API reference pages on Chrome, so that I can quickly learn how to use a specific extension API.

Bonus: A daily extension tip will be displayed as a popover after clicking the "tip" navigation list item.
(Currently, it requires Canary build and the "Experimental Web Platform features" flag enabled. The release date was moved to M114, so this approach may need to be abandoned if we want to launch before then.)

How to use it?

  1. Enter the keyword "api" in the browser address bar.
  2. Then, press "tab" or "space".
  3. Start typing they API keyword.
  4. A list the most common APIs will show or a list of past searches
  5. Select the API desired, and a new page will open to the Chrome API reference page.

TODO:

  • The Popover API is currently being developed. If it's not ready by the launch, then we will make adjustments. The popover was chosen to simplify the example.
  • The ReadME.md will have a link to the tutorial

Copy link
Collaborator

@sebastianbenz sebastianbenz left a comment

Choose a reason for hiding this comment

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

This looks great - just a super quick first pass with a few nits.

functional-samples/tutorial.open-api-reference/content.js Outdated Show resolved Hide resolved
@@ -0,0 +1,106 @@
const apiList = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion: move to a separate JSON file

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 think we are already giving an example of how to fetch with the extension tips feature. I think, if SW supported Import assertions, I would be more open to moving this into a separate JSON file.

How about I move it into a separate JS file? 🧐😁

functional-samples/tutorial.open-api-reference/sw-tips.js Outdated Show resolved Hide resolved
// Send tip to content script via messaging
chrome.runtime.onMessage.addListener((message, sender, sendResponse) => {
if (message.greeting === 'tip') {
chrome.storage.local.get('tip').then(sendResponse);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use async await

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 used .then() here b/c the return value of the onMessage listener determines if Chrome expects sendResponse to be called.

If we use async/await, this function will return a Promise, and Chrome will always expect sendResponse to be called.

We could wrap the call to .get() in an async IIFE, but it seems like overkill for this tutorial😊

Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't know that, have you got a link to where this is explained. Curious to learn how this works.

Copy link
Contributor Author

@AmySteam AmySteam Mar 21, 2023

Choose a reason for hiding this comment

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

Here's the section that explain this, but I think it can be explained better:

// Don't use an async function to handle messages 
// b/c an async function always returns a Promise object
// a Promise object is always truthy 
// regardless of the value that the Promise contains
chrome.runtime.onMessage.addListener((message, sender, sendResponse) => {
    if (message.greeting === "hello") {
        // start a call to get a value from storage
        chrome.storage.local.get("name").then(({ name }) => {
            // this runs after the onMessage handler returns
            sendResponse(`my name is ${name}`)
        })
        // the onMessage handler should return truthy if sendResponse will be called
        return true
    }
    // the onMessage handler should return falsy if sendResponse will not be called
    return false
})

returns boolean | undefined

Screenshot 2023-03-21 at 14 58 50

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks ! That's not intuitive.

One thing: maybe rename the message field from greeting to type as it better expresses what this field is used for.

@AmySteam
Copy link
Contributor Author

@sebastianbenz Thanks for the review. I applied most of your suggestions. I wrote comments on the suggestions I had a few thoughts on.

Copy link
Collaborator

@sebastianbenz sebastianbenz left a comment

Choose a reason for hiding this comment

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

Thanks! Left a few more comments, only nits.

/**
* Returns a list of suggestions and a description for the default suggestion
*/
export async function getApiSuggestions(input) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can you pick a name that better explains what input is?

// Send tip to content script via messaging
chrome.runtime.onMessage.addListener((message, sender, sendResponse) => {
if (message.greeting === 'tip') {
chrome.storage.local.get('tip').then(sendResponse);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't know that, have you got a link to where this is explained. Curious to learn how this works.

@AmySteam AmySteam removed the tutorial label Mar 17, 2023
@AmySteam AmySteam changed the title [New tutorial] Open Chrome API reference page (SW/omnibox API demo) [New sample] Open Chrome API reference page (omnibox, alarms, messaging sample) Mar 17, 2023
Copy link
Collaborator

@sebastianbenz sebastianbenz left a comment

Choose a reason for hiding this comment

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

@jpmedley
Copy link
Contributor

@AmySteam, later today adding a 'latest samples' entry to 'What's new'. It would be cool to include this, but I don't know how much more work you have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants