benwinding / command-pal

The hackable command palette for the web, inspired by Visual Studio Code.
https://benwinding.github.io/command-pal/docs
MIT License
75 stars 8 forks source link

Adding dynamic command seems to not be searchable until after command pallete is opened #11

Open rouilj opened 1 year ago

rouilj commented 1 year ago

Hello:

I am creating a command to copy the current URL to the clipboard. Since the user may not have given me permission, or the clipboard API may not be available, I run some code to determine that the prerequisites are available and add it dynamically.

To do this, I define a commands array and follow along with the documentation:

commands = [ {...}, {...},
      {
      name: "Add new entry here",
      handler: () => (add_new_entry(commands)),
    },
 ];

function add_new_entry (c) {
    c.push( {
        name: "New Command Here",
        handler: () => (alert("I'm a new command!!!") ),
          )
}

const c = new CommandPal({
  hotkey: "ctrl+space",
  commands: commands,
});

function add_copy_url_to_clipboard (c) {
    if ( ! navigator.clipboard ) return;
    navigator.permissions.query({name: "clipboard-write"}).then(
        function (result) {
            if (result.state === "granted") {
                c.push( {
                    name: "Copy URL to Clipboard",
                    handler: () => (navigator.clipboard.writeText(
                        window.location.href)), }
                      )
            }
        }
    )
}

add_copy_url_to_clipboard(commands)

c.start();

If I activate command-pal, I can see the "Copy URL to Clipboard" entry listed, but searching for URL doesn't find it. If I exit (esc key) from command-pal and re-activate it, search for URL works.

Even on the first activation, I can choose the "Copy URL to Clipboard" item by arrowing down to it and it works as expected.

It looks like the dynamic entry is unsearchable only if it is the first invocation after the page loads.

If I choose the "Add new entry here" item (by search or arrow keys), I can see the new entry when I re-activate the command-pal and I can search for "New command here" successfully.

What's weird is even if I change the code to read:

add_copy_url_to_clipboard(commands)

const c = new CommandPal({
  hotkey: "ctrl+space",
  commands: commands,
});

c.start();

searching still fails until the second restart. printing commands to the console shows what I expected.

I'm at a loss to explain this.

With command-pal deactivated, if I call add_new_entry(commands) in the console then activate command-pal, the new item is not there. If I close and re-open command-pal the item is there and is searchable.

Am I doing something wrong?

Console is not reporting any errors.

Browser: Version 109.0.5414.120 (Official Build) (64-bit) on windows 10.

Ideas welcome.

-- rouilj

rouilj commented 1 year ago

I may have partly figured out what's happening here. Because navigator.permissions.query throws me off into promise/async land the definitions of c and calling c.start() may have the commands array changing between the time start is called and the time I display command-pal. Need to figure out how to wait until the promise is resolved before I continue.

rouilj commented 1 year ago

All right I think I have this solved. It looks like URL is searchable. Here is the working code:

commands = [ {...}, {...},
    {
        name: "Add new entry here",
        handler: () => (add_new_entry(commands)),
    },
];

function add_new_entry (c) {
    c.push(
        {
            name: "New Command Here",
            handler: () => (alert("I'm a new command!!!")),
            weight: 22,
        }
    )
}

// sort by optional weight
function sort_by_weight (cmds) {
    console.table(cmds)
    cmds.sort((a,b) => {
        a_weight = a.weight || 0;
        b_weight = b.weight || 0;
        return b_weight - a_weight;
    });
}

async function add_copy_url_to_clipboard (c) {
    if ( ! navigator.clipboard ) return;

    let can_write = navigator.permissions.query({name: "clipboard-write"})

    let result = await can_write

    if (result.state === "granted") {
        c.push(
            {
                name: "Copy URL to Clipboard",
                handler: () => (navigator.clipboard.writeText(
                    window.location.href)),
                weight: 10,
            },
        )
    }
}

// IIFE here to handle async check for clipboard access since you
// can't await at top level until ES 2022.
// Use then to resolve the promise and expose the new command_pal as c.
( async function () {
    add_copy_maybe = add_copy_url_to_clipboard(commands);
    a = await add_copy_maybe

    // put most likely commands at the top
    sort_by_weight(commands);

    const c = new CommandPal({
        hotkey: "ctrl+space",
        commands: commands,
    });

    c.start();
    return { command_pal: c } ;
})().then ( x => c = x.command_pal )

I can use the variable c to allow me to see the inside of command-pal.

One thing I noted is that using c.options.commands.pop() sometimes took a couple of activations of command-pal to actually sync the displayed commands with what was in the array.

rouilj commented 1 year ago

It looks like the command palette is rebuilt in onClose using setItems. But setItems isn't called when opening the palette. So if the addition/deletion occurs before the palette closes, it's picked up immediately.

Otherwise you need to wait for the items to be rebuilt on the next onClose. I suspect this is to prevent a delay by rebuilding the items array when opening (with similar code in multiple places) command-pal.

If minimizing delay is the goal would it be a good idea to change:

  async function onClosed() {
    await asyncTimeout(10);
    if (loadingChildren) {
      return;
    }
    dispatch("closed");
    selectedIndex = 0;
    setItems(inputData);                            // move this below 
    showModal = false;
    if ( ! focusedElement ) {
      console.error("focusedElement not set")
    } else {
      focusedElement.focus()
    }
                                                    // <== move setItems here 
  }

That should:

and allow the user interact with the app before setItems() completes. IIUC async functions run off the main thread. So a long running setItems will run in this async context and not block the main thread or interactivity.

It would be nice for command-pal to auto-update internal state when the commands/inputData is updated, I don't see a way to detect the update of commands with:

Commands is not data wrapped inside a class and is just an array of POJO.

If this is true, command-pal should expose setItems() on the App (maybe using the method in #39?) So the web page can modify commands and run c.setItems() (or c.updateCommands() or whatever).

This would also be useful in an SPA where you could use:

  c.options.commands = c.options.commands.map()

to replace the commands. The first invocation after this operation would result in the old commands array being searched/displayed. At least that explains what I thought I saw at one point, but and too lazy to go back and try it again.