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

Created MV3 Iframe sandBox Example #746

Closed
wants to merge 5 commits into from
Closed

Created MV3 Iframe sandBox Example #746

wants to merge 5 commits into from

Conversation

Ambushfall
Copy link

No description provided.

@google-cla
Copy link

google-cla bot commented Aug 20, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@Ambushfall
Copy link
Author

Port of MV2's iFrame / Notification to MV3

Also added a showcase of a fetch for random JSON endpoint and it's data visible in the console.

Piping manifest.action.default_icon to chrome.action.setIcon results in failure if the HTML is nested.

Fixed by unnesting from the Demo folder.

Hardcoding would also work, but I think this is a bug in chrome, or just lack of code. (Why would you need to be in root to access manifest for action icon?)
@Ambushfall
Copy link
Author

Fixed API/action demo failing to reset the icon.

@AmySteam AmySteam requested a review from dotproto August 22, 2022 14:11
Copy link
Contributor

@dotproto dotproto left a comment

Choose a reason for hiding this comment

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

Thanks for your interest in creating a sample extension. There are a few issues with this PR that will need to be addressed before we can accept your contribution.

At the moment the biggest issue is that the "sandbox" sample introduced by this PR doesn't fit into the kinds of samples we're collecting in this repo. We currently have two main types of samples:

  • The api directory contains extensions that show as much of the API surface of a given namespace as possible in a single demo.
  • The example directory contains fully functional extensions that use a variety of APIs to achieve their goals. That said, they may not be as polished as something you'd find on the Chrome Web Store.

The sample in this PR doesn't use enough of the API surface for the api directory and doesn't do enough to fit in the example directory. I'm also currently considering introducing a third type of sample:

  • The cookbook directory collects functional samples that demonstrate a specific concept or way of accomplishing a given task.

The cookbook approach seems to be the best for your sandbox sample. Before discussing the changes necessary to make this a good cookbook sample, though, there are two things I'd like to get cleaned up:

  1. Please revert all changes to files in api/action. These changes seem unintentional and are out of scope for introducing a new sample.
  2. The sandbox sample contains code that fetches data from a remote server, but this data doesn't meaningfully contribute to the example being presented. Please update the sample to either integrate this data or remove the unused code.

@@ -1,12 +1,12 @@
// Copyright 2021 Google LLC
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert changes to this file. The Action API demo is out of scope for the stated purpose of this PR.

@@ -0,0 +1,22 @@
p {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert changes to this file. The Action API demo is out of scope for the stated purpose of this PR.

@@ -0,0 +1,167 @@
<!DOCTYPE html>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert changes to this file. The Action API demo is out of scope for the stated purpose of this PR.

@@ -0,0 +1,254 @@
// Copyright 2021 Google LLC
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert changes to this file. The Action API demo is out of scope for the stated purpose of this PR.

@@ -1,24 +1,24 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert changes to this file. The Action API demo is out of scope for the stated purpose of this PR.

examples/sandbox/eventpage.js Show resolved Hide resolved
examples/sandbox/manifest.json Show resolved Hide resolved
examples/sandbox/manifest.json Show resolved Hide resolved
examples/sandbox/sandbox.html Show resolved Hide resolved
examples/sandbox/sandbox.html Show resolved Hide resolved
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.

None yet

2 participants