Set link release (no modifier) default to search box

[christian-byrne]

Currently, when releasing link on canvas, the default action is to open the context menu. However, the Litegraph context menus are too difficult to navigate, and the suggestions are misleading about what nodes are actually compatible with the dropped link. The search box has fuzzy searching, filters, a more modern UI, and allows for quicker usage and feedback.

A new user should not be subjected to the context menus as the default link release experience.

[christian-byrne]

I think we should be decisive about these types of design decisions and be okay switching users who have not set a value in the past to the new search box on link release. The new search is a far better experience then the nested LG context menus and also it's something we actually plan on supporting and extending. Perhaps I should seek some other opinions though, because that is jusy my 2 cents. I'll report back after gathering some opinions.

[webfiltered]

First part: Only one of the two settings has been changed. It means that dragging a link with shift held does exactly the same thing as not holding shift (effectively disabling that feature). Is that intended?

Second part: I strongly agree the default should be search, despite its current issues (e.g. search lists every "ANY" node first, so it seems a lot of folks use the context menu "quick" list).

I was just flagging that in case it had not been considered, moreso than taking a stance -- the PR said "new user should", and didn't mention any impact to existing users. I figured we should only change this for new users, rather than silently update existing users.

"What's new" system would make this much easier.

[christian-byrne]

First part: Only one of the two settings has been changed. It means that dragging a link with shift held does exactly the same thing as not holding shift (effectively disabling that feature). Is that intended?

Oh, sorry: I somehow failed to register that aspect of your initial comment. You're right, I should swap them.

[christian-byrne]

I think the subtextual issue here is we don't have a good way to set a setting for new users only. The reason seems to be that settings are only serialized to the user's settings.json if they have set it explicitly. Possible fixes:

1. Just serialize the entire settings object always. Then, if we change a default value, it truly only would be changed for new users. Downside: performance
2. ... can't think of any other solution that isn't extremely complex

I think option 1 is pretty standard. WDYT?

[webfiltered]

Is it standard? I haven't actually noticed that happening in many places.

I think a "not set" value for settings is valuable; e.g. should we ever want to change the default for existing users for whatever-special-reason, we can. "Resetting" a setting should restore it to this state, rather than any explicit-current-default.

But there's a really simple solution imo; check if the file exists.

[christian-byrne]

So we track which settings are unset or check if the file exists, but then how to determine if the setting is unset for a user before this change was made vs. after? A new user starts the app, we can detect they are a new user easily then show the newly changed default value. But then the 2nd time they login, the setting is unset from them and the settings.json file exists, so they just get the old default value again? How will it work exactly?

[webfiltered]

1. Simplest: We set the setting explicitly for existing users when this state is discovered
2. More complex, but allows rollback: set a special flag / value that indicates that this event occurred for this setting (easy for combo list settings - just use a hidden value).

There's no truly clean (edit: and elegant) settings solution for an evolving app that I've ever found.

[christian-byrne]

Hm, that could work. We definitely will need this type of system eventually (e.g., for changing navigation controls).

Would it be easier to do something like this?

  {
    id: 'Comfy.LinkRelease.Action',
    category: ['LiteGraph', 'LinkRelease', 'Action'],
    name: 'Action on link release (No modifier)',
    type: 'combo',
    options: Object.values(LinkReleaseTriggerAction),
    defaultValue: LinkReleaseTriggerAction.CONTEXT_MENU,
+   defaultsByInstallVersion: {
+     '1.21.3': LinkReleaseTriggerAction.SEARCH_BOX,
+     '1.40.3': LinkReleaseTriggerAction.FUTURISTIC_LLM_GIGA_NEW_FEATURE
+   }
  },

Then we just start saving the version in a installedVersion (recording the version of the frontend the user installed with). For anyone who doesn't have that setting, we use the defaultValue.

[webfiltered]

I think that looks cleaner - and will probably(?) make reasoning about it when you need to compare a an actual settings file to the setting impl. easier.