Skip to content

feat: API for creating/manipulating the views hierarchy #32890

Closed
daniel-koc wants to merge 5 commits intoelectron:mainfrom
stack-inc:views_api_proposal
Closed

feat: API for creating/manipulating the views hierarchy #32890
daniel-koc wants to merge 5 commits intoelectron:mainfrom
stack-inc:views_api_proposal

Conversation

@daniel-koc
Copy link
Contributor

@daniel-koc daniel-koc commented Feb 14, 2022

Closes #32751

Motivation

Bring native ScrollView component into Electron to provide flexible way of showing mutliple BrowserViews in one window in a performant way. Please see the issue #32751 for more details.

The challenges we addressed in this PR

  • building a hierarchy of views, in particular, the ScrollView API
  • integration with WebContents API (so far we have used BrowserView)
  • draggable regions feature behavior on Mac and Windows / Linux
  • triggering beforeunload events
  • possible vibrancy for WebContents views

Implementation

For clarity of the review, we organized the code by several commits and is best reviewed commit-by-commit:

  1. 18cf919e7 - The implementation of BaseView which is the base class for all views.

    • documentation base-view.md
    • api::BaseView
    • implementation of native layer NativeView (Cocoa for Mac, Chromium views for Windows/Linux).

  2. 132a9992d - The few additional functions for BaseWindow/BrowserWindow API.

    • addChildView
    • removeChildView
    • setContentBaseView
    • also implements triggering beforeunload events for views hierarchy / updating draggable regions and few more.

  3. 0c13af938 - The implementation of ContainerView which allows building the hierarchy of views.

    • documentation container-view.md
    • api::ContainerView
    • and NativeContainer.

  4. 2cc9ed017 - Implementation of ScrollView

    • documentation scroll-view.md
    • api::ScrollView
    • and NativeScroll.

  5. 53f0b0e72 - The wrapper for BrowserView and some additional changes for BrowserView (window/view owner).

API

An example code will look the following

const { BaseWindow, ContainerView, ScrollView, BrowserView } = require("electron");

const window = new BaseWindow({ width: 1000, height: 500 })

const scroll = new ScrollView()
scroll.setBounds({ width: 1000, height: 500, x: 0, y: 0 })

const scrollContent = new ContainerView()
scrollContent.setBounds({ width: 2000, height: 500, x: 0, y: 0 })

const browserView1 = new BrowserView()
browserView1.setBounds({ width: 1000, height: 500, x: 0, y: 0 })
scrollContent.addBrowserView(browserView1)

const browserView2 = new BrowserView()
browserView2.setBounds({ width: 1000, height: 500, x: 1000, y: 0 })
scrollContent.addBrowserView(browserView2)

scroll.setContentView(scrollContent);
win.setContainerView(scroll);

@welcome
Copy link

welcome bot commented Feb 14, 2022

💖 Thanks for opening this pull request! 💖

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add app.isPackaged() method
  • docs: app.isDefaultProtocolClient is now available on Linux

Things that will help get your PR across the finish line:

  • Follow the JavaScript, C++, and Python coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made following the documentation styleguide.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label Feb 14, 2022
@daniel-koc daniel-koc marked this pull request as ready for review February 14, 2022 17:31
@daniel-koc daniel-koc force-pushed the views_api_proposal branch from 53f0b0e to 225a325 Compare March 4, 2022 21:56
@daniel-koc daniel-koc force-pushed the views_api_proposal branch from 225a325 to c75a112 Compare March 5, 2022 18:07
@daniel-koc daniel-koc force-pushed the views_api_proposal branch from c75a112 to 6305cea Compare March 5, 2022 19:40
@nornagon
Copy link
Contributor

nornagon commented Mar 14, 2022

Hi @daniel-koc, thanks for the PR! I think this proposal should go through the API WG's RFC process. There isn't really consensus on how we should approach exposing Views. This is the second attempt at it (you can see part of the first one here), and I think we would benefit by going through a more rigorous design process this time around. Once you have a spec document, we can meet with the API WG to discuss and move the proposal forward.

Let me know if you have any questions!

@nornagon nornagon self-assigned this Mar 14, 2022
@felixrieseberg
Copy link
Member

felixrieseberg commented Mar 25, 2022

Hey @nornagon! There's an RFC up here - let us know if there's additional information or other tradeoffs you're interested in!

@ckerr ckerr added the semver/major incompatible API changes label Jun 22, 2022
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Jun 22, 2022
@nornagon
Copy link
Contributor

nornagon commented Sep 13, 2022

Marking this as wip since it's currently in a quite different state from the RFC.

@nornagon nornagon mentioned this pull request Sep 22, 2022
Merged
10 tasks
@estoykov
Copy link

estoykov commented Nov 14, 2022

Hi,

It is very nice to see that you're constantly improving Electron, making it more flexible and comfortable to use :)
And this WebContentsView and the while new ability to have hierarchical views structure is very neat ;)

It seems that Chromium views hierarchy (Z order I think - used for input and composition) is maintained using the simple list order - the last in the views list has the highest top Z order.
And if this applies for the proposed Electron views as well, then at least we could specify initial view's Z order - using "index" parameter in "addChildView" method.
However, there is no method to change Z order dynamically - I don't see a method like the previous BrowserWindow.setTopBrowserView for example.
Implementation of BrowserWindow.setTopBrowserView seems to (almost) just remove/add the view to put to the end of list, as this defines the Z order as it seems.

So, is it possible to add a similar method somewhere in this new WebContentsView components?

Thank you in advance and keep the good work!
Cheers!

@samuelmaddock
Copy link
Member

samuelmaddock commented Jan 13, 2025

@daniel-koc The Views API has changed substantially since this PR was proposed. I wanted to check in to see if there's still interest in pursuing aspects of the proposed changes in this PR. The API working group can continue to review if so.

@samuelmaddock
Copy link
Member

samuelmaddock commented Jan 31, 2025

Thank you for your contribution to Electron!

We've noticed there hasn't been any activity on the pull request in some time, and there have been many changes to the codebase since it was opened. As a result, your pull request may no longer be applicable or could conflict with more recent updates.

To keep pull requests relevant for maintainers, we'll be closing this one for now. However, if you believe it's still relevant or if there was a mistake, feel free to request us to reopen it or provide a follow-up comment.

If you'd like to continue with your contribution, please update your branch with the latest changes from main and resolve any conflicts.

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

Reviewers

No reviews

Assignees

@nornagon nornagon

Labels

api-review/requested 🗳 enhancement ✨ semver/major incompatible API changes stale wip ⚒

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FeatureRequest]: native ScrollView in the main process

6 participants

@daniel-koc @nornagon @felixrieseberg @estoykov @samuelmaddock @ckerr