airbnb / react-sketchapp

render React components to Sketch ⚛️💎
http://airbnb.io/react-sketchapp/
MIT License
14.95k stars 824 forks source link

Render script runs in first open document regardless of window focus #23

Closed daneden closed 4 years ago

daneden commented 7 years ago

Expected behavior: running npm run render should run the script in the currently open/focused document.

Observed behavior: running npm run render runs the script in the first document opened in the current window/session.

How to reproduce:

  1. Open or create two Sketch documents
  2. Focus the second document window/tab
  3. Run npm run render
  4. The second document is unchanged; the first, unfocused document is where the contents are rendered.

Sketch version: 43.2

jemgold commented 7 years ago

Ooh interesting — I think this might be a hard limitation with the current version of sketchtool, unfortunately.

λ sketchtool run
Missing arguments for command ‘run’.

Usage: sketchtool run <bundle> <command> [ --application=<path> | -A <path> ] [ --new-instance{=YES|NO} | --no-new-instance | -N {<YES|NO>} ] [ --wait-for-exit{=YES|NO} | --no-wait-for-exit | -W {<YES|NO>} ] [ --context=<string> | -C <string> ]

Run a command from a plugin, inside Sketch.

Arguments:

    bundle            plugin bundle containing the command to run
    command           the command to run

Options:

    --application     The version of Sketch to launch. If not supplied, the default is to use the version containing sketchtool. (optional).
    --new-instance    Launch a new instance of Sketch, even if one is already running. (optional, defaults to NO).
    --wait-for-exit   Wait for Sketch to exit before returning from this command. (optional, defaults to NO).
    --context         JSON dictionary of values to pass in to the plugin. (optional).

cc @bomberstudios

mathieudutour commented 7 years ago

Is it? I would guess it's coming from context.document here: https://github.com/airbnb/react-sketchapp/blob/master/examples/basic-setup/src/my-command.js#L71

You can access all the documents with NSApplication.sharedApplication().orderedDocuments(). Here is a small function (not tested so probably doesn't work) to get the focused document or create a new one if there is nothing opened yet:

function getMainDocument (context) {
    const documents = NSApplication.sharedApplication().orderedDocuments()
    if (!documents || !documents.length) {
        return context.api().Application(context).newDocument()
    }
    return documents.find(d => d.documentWindow().isMainWindow()) || documents[0]
}
daneden commented 7 years ago

@mathieudutour that snippet still isn’t working, sadly. Because the application is in the background when the script runs (assuming it's being run from the command line or after a .js file is changed), all the documents return false for isMainWindow(), which once again causes the script to run in the first open document.

@jongold it looks like sketchtool's --context argument may come in handy here. The render script runs skpm build --run, but --run seems to run the plugin without context. Perhaps the render script can instead build the plugin using skpm and then run it using sketchtool with provided context?

jemgold commented 7 years ago

🤔 if we can fix this with a patch to skpm that would be the best. I'd also love reusable helpers for 'mainDocument' / 'documentByName' / etc, jQuery style. is that feasible? sketchtool run was added recently and I still don't full understand how context works :)

mathieudutour commented 7 years ago

all the documents return false for isMainWindow()

🤔 hum I didn't know it was possible to not have a main window. I'm not a cocoa expert by no mean so there might be a way that I don't know about

I think the context option is just to pass some arguments to the plugin, not sure how we could use it. And as long as we are using context.document in the render, skpm won't be able to do anything

jemgold commented 7 years ago

you can pass whatever you want into render -

if we added a 'target' to 'context' you could equally do render(<Foo />, context.target)

mathieudutour commented 7 years ago

yes that's what I mean, we need to change the examples. but target would be very react-sketchapp specific, not sure we want to bake that into skpm. But what we can do is allow to pass the context as an argument to the --run option of skpm.

Still, I don't know what we can possibly pass.

daneden commented 7 years ago

Ack, so my assumption was wrong. I thought that context.document would point to the current active document even with a backgrounded window, but it, too, always points to the first open document.

thecalvinchan commented 6 years ago

is there documentation for what context contains? I'm trying to work with this issue but I can't seem to find any documentation anywhere

macintoshhelper commented 4 years ago

Hi, this seems to be resolved now. render renders to the current/last active window.

@thecalvinchan context is being deprecated afaik.

You can go to the menu bar -> Plugins -> Run Script -> and run console.log(context);


Here's some sample code of finding a document by name, and falling back to creating a new document if it doesn't exist.

const sketch = require('sketch');

const getDocumentByName = name => {
  return (sketch.getDocuments() || []).find(doc => {
    const nativeDoc = doc.sketchObject || {};
    // (native API)
    const docName = nativeDoc.displayName ? nativeDoc.displayName() : '';
    if (docName.trim() === name) {
      return doc;
    }
  });
};

export default () => {
 const doc = getDocumentByName('My Design System') || new sketch.Document(); // create new document if exact document not found

  render(<App />, doc);
}