Provider modules #32

seanoliver · 2023-06-08T18:13:11Z

@swyxio Not ready for a PR yet but wanted to share some direction in case I'm taking this too far haha. I refactored the provider-specific code into subclasses so that we can more easily swap one in out for another.

You probably already know this but since I'm new to Electron I found out that we need to keep JS in the render process to access the providers' DOMs. So we can't use Node.js imports/requires to organize the code in separate files. Argh.

If this directionally looks good my next steps will be to add another provider and a setting that saves you preferred providers to local disk. Also the grabby bars between panes aren't working on this version so I'll fix that before submitting.

swyxio · 2023-06-09T16:04:23Z

interface.js

@@ -0,0 +1,436 @@
+console.log('interface.js loaded');
+const Store = require('electron-store');


it looks like nodejs imports are fine here?

Indeed! Not sure what was going on the other day — imports are working fine so i split out each provider class to it's own file.

swyxio · 2023-06-09T16:05:07Z

interface.js

+			document.addEventListener('paste', (event) => {
+				event.preventDefault();
+				let text = event.clipboardData.getData('text');
+				let activeElement = document.activeElement;
+				let start = activeElement.selectionStart;
+				let end = activeElement.selectionEnd;
+				activeElement.value = activeElement.value.slice(0, start) + text + activeElement.value.slice(end);
+				activeElement.selectionStart = activeElement.selectionEnd = start + text.length;


i somewhat worry about overriding paste like this, lets make sure to check it works in the main chat apps

swyxio · 2023-06-09T16:05:41Z

interface.js

+				activeElement.selectionStart = activeElement.selectionEnd = start + text.length;
+				});
+			`);
+			}.bind(this)


fun js trivia - how to rewrite your function so you dont have to .bind?

swyxio · 2023-06-09T16:07:54Z

interface.js

+class OpenAi extends Provider {
+	static webviewId = 'webviewOAI';
+	static webview = document.getElementById('webviewOAI');
+


prob wanna give it a "display name" too since we'll probably be showing a dropdown

swyxio · 2023-06-09T16:08:31Z

interface.js

+/* END Providers ------------------------------------------------------------ */
+
+/* ========================================================================== */
+/* Create Panes                                                               */
+/* ========================================================================== */


good comments

swyxio · 2023-06-09T16:09:09Z

interface.js

+let paneProviders = [Bard, OpenAi, Claude];
+
+// Create the panes based on the mapping
+paneProviders.forEach(provider => {
+
+	const providerPane = document.getElementById(`${provider.name.toLowerCase()}Pane`);
+	providerPane.classList.remove('hidden');
+
+});


i wonder how this will look with a 4th provider.. (Bing)

interface.js

swyxio · 2023-06-09T16:11:24Z

interface.js

+// Adjust styling for enabled providers
+Bard.handleCss();
+OpenAi.handleCss();
+Claude.handleCss(Claude.webview);
+
+// fix double-pasting inside webviews
+OpenAi.setupCustomPasteBehavior();
+Bard.setupCustomPasteBehavior();
+Claude.setupCustomPasteBehavior();


this feels like "setup code" that should belong higher up, because the other stuff directly above this is oninput and onsubmit code

swyxio

looking good so far!

seanoliver · 2023-06-13T07:53:08Z

@swyxio I think with these changes we're very close (assuming it's working on your end) — i noticed another PR open for smoltalk is started but not too much progress yet. This one feels like a significant enough refactor that you might want to merge it in sooner rather than later since modularizing the providers required touching so many parts of the app.

Once it's merged in, I can submit another PR for additional providers, without blocking your/others' work on other parts of the app.

Resizing and Keyboard Shortcuts:

Grabby bars and resizing are working again (added some subtle hover styling just to give more of a hint that they're interactive)
Cmd +/- are working but the Cmd 1/2/3 are not (I'm not sure what the desired behavior actually is for that one -- do we want it to disable all other panes and show only one or do we want it to be used in combination with +/- to zoom only a specific pane?)

Moved Preferences to Context Menu:

BIG CHANGE: Feel free to reject this if you don't like it but i put the preferences into the electron dropdown menu and got rid of the popup for two reasons: (1) the preferences window wouldn't stay on top after checking something on / off (2) having to "save" your preferences with an explicit button click seems a little less common now days vs. just toggling things off from the context menu. Also as we add more providers the window will need to keep getting taller for the button to stay in view — otherwise people are likely to miss it and think the checkboxes aren't working.
There's 4 providers in this version, so people could reasonably choose up to 4 and it would be tight but probably okay per your note above.
Once we add more providers, I was thinking we could just disable other providers once 4 are checked, so they'd need to uncheck one if they want to check more. (That's assuming we cap at 4 and don't stack two rows which is also a possibility with split.js!)

If you can clarify the Cmd 1/2/3 behavior I can push a final change to get this merged! This feels like a big update!

zleman1593 · 2023-06-13T22:21:25Z

I'm getting an error after opening the built app for arm. This does not happen on the main branch.

seanoliver · 2023-06-13T22:58:09Z

@zleman1593 can you try running npm install or npm install electron-log? I added a logging package for development in this version so that's a new dependency.

@swyxio happy to remove this for the release, too, but having terminal logging for the render process has been super helpful so probably makes sense to at least keep in the dev environment!

zleman1593 · 2023-06-13T23:00:34Z

I did both of those steps before publishing the above. Unfortunately, neither worked.

seanoliver · 2023-06-13T23:06:46Z

@zleman1593 thanks for flagging this -- I can reproduce this and will update here once we have a fix!

seanoliver · 2023-06-13T23:10:31Z

@zleman1593 when you have a second can you clone this latest version and see if it's working for you? it looks like it was that the package was missing from package.json

zleman1593 · 2023-06-13T23:42:04Z

That worked.

swyxio · 2023-06-15T00:46:13Z

@seanoliver amazing - grabby bars works!

i only see 3 windows, not 4. what is the default supposed to be?

smol bug with cmd+1/2/3/A - also refreshes the screen for some reason.

cmd+1/2/3 basically shrinks one window and maximizes one window to one specific chat, or cmd+A to restore equal width for all chats.

lastly, checking these disable buttons while they are actually enabled is unintuitive. would massage the wording or flip the checkbox state here

seanoliver · 2023-06-15T02:55:13Z

@swyxio Ah thanks I realize now I could have just run the last release version to confirm the behavior!

All 4 providers should be enabled by default now
Pressing Cmd/Ctrl + (pane number starting from 1) will minimize the other panes and expand the indicated pane to 100%
Pressing Cmd/Ctrl + 'a' or 'A' will distribute them evenly again (only among the enabled providers)
Removed the "enabled" or "disabled" language in the dropdown so the checkbox indicates if a provider is enabled
Fixed the bug where we were refreshing/redrawing the panes with the keyboard shortcuts.

Lmk if this is working for you! If so I think we can ship Bing and I'll start a branch to add more providers. 🙌

swyxio · 2023-06-15T07:42:35Z

wooohoo!

seanoliver added 10 commits June 7, 2023 17:41

moved js code to interface.js

9ac66dd

indentation

528f566

indentation

ed52b55

encapsulating provider-specific functions

7044f8d

clean this up

6cf672e

refactored panes code to handle configurable providers

1d46b0d

hidden class

53b2c7f

unhide active providers

0caf8d9

load provider panes in render

b6d51f9

removing provider specific files

2125975

seanoliver mentioned this pull request Jun 8, 2023

[Feature] make window of each URL customizable #13

Closed

swyxio reviewed Jun 9, 2023

View reviewed changes

interface.js Outdated Show resolved Hide resolved

swyxio reviewed Jun 9, 2023

View reviewed changes

interface.js Outdated Show resolved Hide resolved

swyxio reviewed Jun 9, 2023

View reviewed changes

seanoliver added 9 commits June 9, 2023 13:44

refactored out binds with arrow funcs, code cleanup

010e9d6

add comment

c70ca6e

remove claude webview args

3b3e275

adding some more explanatory comments

21746c6

adding bing pane to main application view

2d6066c

adding edge user agent to bing webview

283f44f

moved provider classes into standalone files

b89fa9b

adding bing -- css is wip

53c9c72

refactoring out the provider classes

4624826

seanoliver added 8 commits June 12, 2023 00:17

draw panes dynamically based on enabled status

f569f20

add getWebview method

ae314c1

added fullName static property to provider classes

1330904

moved preferences into dropdown menu

5ec1503

slightly nicer import style

5704082

created drawPanes function and some logging statements

a52f34f

adding comments to explain things

e6b35d4

adding moar comments explaining logic

80609dc

adding electron log to package.json

5bac682

set default zoom level to 0 across all panes

4e694a2

seanoliver added 3 commits June 14, 2023 19:27

fix pane resize keyboard shortcuts

fb8c5f3

enable claude by default

7927e49

cleaned up enabled disabled state in menubar prefs

3b4c49b

quick fix to default checkbox to false if not set

d679b1f

swyxio merged commit bc98204 into smol-ai:main Jun 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provider modules #32

Provider modules #32

seanoliver commented Jun 8, 2023

swyxio Jun 9, 2023

seanoliver Jun 10, 2023

swyxio Jun 9, 2023

swyxio Jun 9, 2023

seanoliver Jun 9, 2023

swyxio Jun 9, 2023

swyxio Jun 9, 2023

swyxio Jun 9, 2023

swyxio Jun 9, 2023

swyxio left a comment

seanoliver commented Jun 13, 2023

zleman1593 commented Jun 13, 2023

seanoliver commented Jun 13, 2023

zleman1593 commented Jun 13, 2023

seanoliver commented Jun 13, 2023

seanoliver commented Jun 13, 2023

zleman1593 commented Jun 13, 2023

swyxio commented Jun 15, 2023 •

edited

Loading

seanoliver commented Jun 15, 2023

swyxio commented Jun 15, 2023

		@@ -0,0 +1,436 @@
		console.log('interface.js loaded');
		const Store = require('electron-store');

Provider modules #32

Provider modules #32

Conversation

seanoliver commented Jun 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swyxio left a comment

Choose a reason for hiding this comment

seanoliver commented Jun 13, 2023

zleman1593 commented Jun 13, 2023

seanoliver commented Jun 13, 2023

zleman1593 commented Jun 13, 2023

seanoliver commented Jun 13, 2023

seanoliver commented Jun 13, 2023

zleman1593 commented Jun 13, 2023

swyxio commented Jun 15, 2023 • edited Loading

seanoliver commented Jun 15, 2023

swyxio commented Jun 15, 2023

swyxio commented Jun 15, 2023 •

edited

Loading