Skip to content

Support AI Action launch for Wami Web App #60

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 10 commits into from
Apr 25, 2025

Conversation

runyuany
Copy link
Contributor

Support AI Action launch for Wami Web App

support share target api

hack to support the image handler

bugfix

bugfix

add

debug

navigate and use the existing flow if already created

add protocol handler

support ai action steps

add log to sw

Revert "add log to sw"

This reverts commit ea32858.

change the bucket name to hard-coded one

remove the slash in title

add actions manifest
wami/sw.js Outdated
@@ -99,6 +99,58 @@ self.addEventListener('activate', event => {
// string. So we need to add it to match the cache.
self.addEventListener('fetch', event => {
const url = new URL(event.request.url);

// Handle share target requests
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update the VERSION const at the top of the file to v9. This will make sure the SW is updated in existing clients.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks for reminding.

wami/sw.js Outdated
@@ -99,6 +99,58 @@ self.addEventListener('activate', event => {
// string. So we need to add it to match the cache.
self.addEventListener('fetch', event => {
const url = new URL(event.request.url);

// Handle share target requests
if (event.request.method === 'POST' && (event.request.url.includes('/share-target'))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we try to keep the main fetch handler untouched and move this into another fetch handler that's dedicated to share target requests?

So, keep the existing self.addEventListener('fetch', event => { ... }); unchanged, but add a second one that contains the code you're adding here.

It's easier to read the file this way, I find. And this way you can also add some comments before the second listener, to explainer what it does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Moved the share target logic into another fetch handler.

wami/app.js Outdated
}

// Simply log any protocol activation
function checkProtocolActivation() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function checkProtocolActivation() {
function logProtocolActivation() {

wami/app.js Outdated
if (protocolUrl && protocolUrl.startsWith('web+wami:')) {
console.log(`Protocol activation detected: ${protocolUrl}`);

// Just clean up the URL without any further processing
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Just clean up the URL without any further processing
// Clean up the page URL.

I like removing words like "just" or "simply" because they tend to send the wrong signal that things are simple, which we can't assume is the case for everyone reading the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, rephrased the comments to make them more straightforward and clear.

wami/app.js Outdated
const urlParams = new URLSearchParams(window.location.search);
const isShared = urlParams.get('share') === 'true';

if (isShared) {
Copy link
Contributor

Choose a reason for hiding this comment

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

When almost the entire body of a function is wrapped in an if statement like here, it's nice to reverse the condition and do an early return. This avoids having to indent the entire function body.
So, instead of if (something) { .... the rest of the function .... }, can you please do if(not something) { return; } ... then the rest of the function, unindented ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored, thanks.

wami/app.js Outdated
const shareCache = await caches.open('share-target-cache');
const shareDataResponse = await shareCache.match('shareData');

if (shareDataResponse) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here, I think we can do an early return if !sharedDataResponse instead of wrapping many lines below into this if.

wami/app.js Outdated
];

// If URL field exists and starts with web+wami://, use it as the flow name
if (shareData.url && shareData.url.trim() !== '') {
Copy link
Contributor

Choose a reason for hiding this comment

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

The code in this function is pretty long and a bit hard to read. Can you maybe extract some of its subparts into other functions, so that the main flow is easier to read?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added several helper functions to make the code block more readable.

@diekus diekus merged commit f5a23ed into MicrosoftEdge:main Apr 25, 2025
1 check passed
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.

3 participants