Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create WebViewProvider API #177

Closed
tjcouch-sil opened this issue May 10, 2023 · 8 comments · Fixed by #225, paranext/paranext-extension-sneeze-board#6, paranext/paranext-extension-template#19 or #242
Assignees

Comments

@tjcouch-sil
Copy link
Member

tjcouch-sil commented May 10, 2023

We need to change the way the web view api works so the renderer can reach out and get web views from the backend for the following benefits:

  1. Changing layouts is possible
  2. WebViews can have persistent state
  3. Possibly embed WebViews in each other
  4. WebViews persist through refreshing the page
  5. Can open multiple of the same type of WebView if the extension allows

We need a WebViewProvider (or WebViewDeserializer or something) API similar to VSCode's WebViewPanelSerializer (example) that is a network object that has a method to deserialize a webview (aka provide the information needed to create the webview. Primarily the react function component or the html page) based on the state given to it (if we want to do this state thing).

Then the web view service would run these when it is loading its layout, then pass them into the dock layout react component once it gets the results.

The general flow would be according to the following very rough design diagram:

Image

It seems to make sense also that we would have some web view service function or something that creates a panel with a specified web view type (similar to the current addWebView command, but you don't have to provide the contents. Just the panel/layout properties and the webview type). In that, the webview service would reach out to the appropriate WebViewProvider to get the actual web view information to display it.

@tjcouch-sil tjcouch-sil converted this from a draft issue May 10, 2023
@tjcouch-sil tjcouch-sil moved this to 📋Product Backlog in Paranext May 10, 2023
@tjcouch-sil tjcouch-sil added this to the External Extension Ready milestone May 10, 2023
@tjcouch-sil tjcouch-sil moved this from 📋Product Backlog to 🏗 In progress in Paranext May 26, 2023
@tjcouch-sil tjcouch-sil self-assigned this May 26, 2023
@tjcouch-sil
Copy link
Member Author

tjcouch-sil commented Jun 2, 2023

To do:

  • No more including content in OnDidAddWebView event
  • No more including content in webViewService.addWebView
  • Web View Provider provides content for a WebView
  • Types distinguish different kinds of WebViews
  • Some way to open a new webview
  • WebViews persist through refresh
  • WebView state?

@tjcouch-sil
Copy link
Member Author

WebView state split off to #222

@tjcouch-sil
Copy link
Member Author

Created pull request on rc-dock for patch applied to enable finding tabs by webViewType ticlo/rc-dock#208

@irahopkinson
Copy link
Contributor

After this was first merged (PR #225), we see a number of errors in the browser console:
Image

@irahopkinson irahopkinson reopened this Jun 15, 2023
@irahopkinson irahopkinson moved this from ✅ Done to 🔖 Open in Paranext Jun 15, 2023
@tjcouch-sil
Copy link
Member Author

After this was first merged (PR #225), we see a number of errors in the browser console: Image

I have seen this recently as well in Paranext and in developing my own application. I think it is possible that the owner of the api added throttling to his api, and we are hitting that throttle limit. We may want to reach out to the owner of the api to discuss the situation with him, just stop using the api (maybe @lyonsil could implement the data provider in C# instead ;) ), or temporarily host our own (seems like overkill). It may be worth discussing what we want to do moving forward. Could we close this issue again and open a new issue about figuring out what to do about bible.api?

@irahopkinson
Copy link
Contributor

It could just be a throttling issue with the site we are hitting. However, the first error is a CORS issue and it would surprise me if throttling caused that (but it might). If we can demonstrate that it's just a throttling error then this issue is done. Do you have any thoughts on how we could do that? Like, do you get those errors when you are completely offline? If you do and you go back to main before this PR was merged do we still see the same errors when offline?

@irahopkinson
Copy link
Contributor

Hmm... as a data point, I just ran up Rolf's PR#245 and didn't see any of those errors at first. However a restart (trivial change to main.ts) produced one set of the 3 errors. A full restart of npm start produces the errors again.

@darren8c darren8c moved this from 🔖 Open to ✅ Done in Paranext Jun 21, 2023
@tjcouch-sil
Copy link
Member Author

Created #258 for CORS issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment