latter-bolden / port

A ship runner and manager for Urbit OS
180 stars 12 forks source link

fix copy url keyboard shortcut #105

Closed ghost closed 3 years ago

ghost commented 3 years ago

fixes #93

Looks like the issue was the shortcut handler for "cmd + L" references a Nativefier helper function that didn't get copied over.

I just inlined the functionality, but alternatively, we could pull in the whole set of helpers if you'd prefer. The file relevant file is here.

arthyn commented 3 years ago

Hey there, thanks for attempting this one! So I did copy most of those helpers including the getCurrentUrl but placed them in main-window.ts which may not have been the most clear.

Because we're using browser views for Landscape, we actually need to retain using the passed in getCurrentUrl and make a few edits in main-window.ts. It doesn't look like I can push to your fork so here's the edits:

  const withFocusedView = <T>(block: (contents: WebContents) => T | undefined, target: 'window' | 'view' = 'view'): T | undefined => {
    const focusedWindow = BrowserWindow.getFocusedWindow();
    if (focusedWindow) {
      const contents = getWindowOrViewContents(focusedWindow);
      return target === 'window' ? block(focusedWindow.webContents) : block(contents)
    }
    return undefined;
  };
  const getCurrentUrl = (): string => {
    return withFocusedView((contents) => contents.getURL());
  }
ghost commented 3 years ago

Oof, could have sworn I grepped for getCurrentUrl, but I see now it's injected into createMenu. Not sure how I missed that, apologies for the sloppy tinkering.

Any who, I updated the helpers as you noted. I also see how the url's differ between the BrowserWindow and BrowserView in Landscape, so the wrapped version makes sense to me now.