BrowserView: Updated, rebased PR with bug fixes. #831
Conversation
|
@kanishk98 is this good to merge? If yes, then I'll need to cross-check the auto-update before merging. |
|
Yeah, features and settings seem to be working pretty well for me. I'll do another check just to make sure, but I think it's good to be merged. Please also text me on czo if you need any help with testing the auto-update process. |
|
Okay, great. I'm testing the auto-update on macOS first. Can you do the same on Linux? |
I can test it out on Windows (will let you know the results on czo). |
View extends the BrowserView class in the main process and provides and interface similar to webview. Other functions can be added later when required.
Adds ViewManager class which makes it easier to create, manage and delete views. Also, created some basic listeners which will be required later.
Removes all instances of webview from main.ts and adds an initial working instance of BrowserView.
Implements logic for updating badgeCount to show message count in sidebar.
Removes tooltips that we show for orgs and settings, as these tooltips are no more visible over the BrowserViews, so there is no use of keeping them. Instead, uses title property of HTMLElements, which can be displayed over BrowserViews.
Destroys all views to recreate them later when app is reloaded. Also, set lastActiveIndex to 0 when a tab is removed.
Add logic to set user agent for each browserview. Also, the badgeCount was not shown initially, on opening the app. Adds that too.
Adds a refresh function which refreshes view every 200 ms, and shows it only if the view DOM content is loaded. Also, gets rid of buggy browser view switching which we were facing earlier.
kanishk98
force-pushed
the
bv-migration
branch
from
8487ac5 to
3770884
Compare
|
I've fixed the merge conflicts and also updated the broken link to the modified |
|
Oh, that error is really frustrating. I'll check it out. |
Removes the heavy setInterval which we were using earlier to prevent heavy CPU Usage. The main reason we were using a setInterval earlier was because view switching was buggy. This commit fixes that.
kanishk98
force-pushed
the
bv-migration
branch
from
3770884 to
20708a2
Compare
|
@akashnimare @kanishk98 what's the current status on this ? |
|
@vsvipul hey, good to see you back :) This is next in our roadmap. I'm releasing the stable release from the master, once done we can push a beta release with this PR. |
|
Heads up @kanishk98, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
|
@manavmehta let's clean it up and merge. |
| showNetworkError(): void { | ||
| this.webview.forceLoad(); | ||
| } |
There was a problem hiding this comment.
@kanishk98 @akashnimare Do we need to totally remove this or need to create some alternative?
What's this PR do?
Same purpose as #793, but updated to pass tests and include more recent features.
For code to pass compilation, we may have to modify
electron.d.tsto the file here.I needed to make the above changes so that
page-title-updatedwould be counted as abefore-input-event.You have tested this PR on: