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

Refactor NativeWindow (Part 13): Add View and WebContentsView APIs #12858

Merged
merged 7 commits into from May 15, 2018

Conversation

Projects
None yet
2 participants
@zcbenz
Contributor

zcbenz commented May 8, 2018

Now BrowserWindow works by using the new View APIs:

BrowserWindow.prototype._init = function () {
  this.setContentView(new WebContentsView(this.webContents))
}

The View API is not yet ready for public usage (it crashes in some cases), and I'm still exploring the best interface. This PR is mostly for refactor purpose.

However you can still play with the View API a bit:

const {TopLevelWindow, WebContentsView, webContents} = require('electron').remote
const web = webContents.create({})
const view = new WebContentsView(web)
const win = new TopLevelWindow({})
win.setContentView(view)
web.loadURL('http://github.com')

@zcbenz zcbenz requested a review from electron/reviewers as a code owner May 8, 2018

@electron electron deleted a comment May 8, 2018

@nornagon

Is it possible to write a test for a window that has a View content view (as opposed to a WebContentsView)?

Show outdated Hide outdated atom/browser/api/atom_api_browser_window.cc
namespace api {
View::View(views::View* view) : view_(view) {}

This comment has been minimized.

@nornagon

nornagon May 11, 2018

Contributor

😂

@nornagon

nornagon May 11, 2018

Contributor

😂

@@ -60,8 +56,7 @@ class NativeWindow : public base::SupportsUserData,
void InitFromOptions(const mate::Dictionary& options);
virtual void SetContentView(
brightray::InspectableWebContents* web_contents) = 0;
virtual void SetContentView(views::View* view) = 0;

This comment has been minimized.

@nornagon

nornagon May 11, 2018

Contributor

Cool!!

@nornagon

nornagon May 11, 2018

Contributor

Cool!!

@zcbenz

This comment has been minimized.

Show comment
Hide comment
@zcbenz

zcbenz May 15, 2018

Contributor

Is it possible to write a test for a window that has a View content view (as opposed to a WebContentsView)?

The API still have a few crash cases when used independently, I will add tests when I fixed them. This PR is mostly for refactoring purpose.

Contributor

zcbenz commented May 15, 2018

Is it possible to write a test for a window that has a View content view (as opposed to a WebContentsView)?

The API still have a few crash cases when used independently, I will add tests when I fixed them. This PR is mostly for refactoring purpose.

zcbenz added some commits May 8, 2018

give window a default content view
Certain APIs are expecting the window to have a content view, having a
default one simplifies our design.

@zcbenz zcbenz merged commit 9fc6b9d into master May 15, 2018

11 checks passed

WIP ready for review
Details
ci/circleci: electron-linux-arm Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm-test Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64-test Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-ia32 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-x64 Your tests passed on CircleCI!
Details
ci/circleci: electron-mas-x64 Your tests passed on CircleCI!
Details
ci/circleci: electron-osx-x64 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@zcbenz zcbenz deleted the api-view branch May 15, 2018

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