8000 refactor!: remove unused `ImageView` browser module by ckerr · Pull Request #46607 · electron/electron · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

refactor!: remove unused ImageView browser module #46607

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

Closed
wants to merge 1 commit into from

Conversation

ckerr
Copy link
Member
@ckerr ckerr commented Apr 10, 2025

Description of Change

BREAKING CHANGE

We should either remove or document the ImageView browser module. It's been around for several years now and we pay maintenance costs on it, e.g. in the Upgrades WG, but it's never been public API.

My preference would be to remove 😈 , since you don't have to do maintenance on code that doesn't exist. But maybe people would use it if we documented it? When I mentioned in #ask-anything that this was unused, @nikwen opened a docs ticket suggesting that. 👍

So this sounds like a decision for @electron/api-wg

(Technically it's not a breaking change to remove undocumented API, but I think we do have prior history on listing those changes as breaking Just In Case someone came across the feature on their own and is using it anyway, so that's why I'm listing this as a breaking change in this PR)

Checklist

Release Notes

Notes: Removed undocumented ImageView browser module.

BREAKING CHANGE

In case any users came across this feature somehow and are using
undocumented API, I'm marking this as a breaking change to ensure
it gets noted in the release notes.
@ckerr ckerr added semver/major incompatible API changes no-backport labels Apr 10, 2025
@ckerr ckerr requested a review from a team as a code owner April 10, 2025 14:58
@nikwen
Copy link
Member
nikwen commented Apr 10, 2025

Adding context: The purpose of the API was to enable splash screens that will be swapped for WebContentsView when the content finishes loading.

Usage example: https://gist.github.com/itsananderson/fc8b735fdf19714eee1d39e2c427b7e3

Relevant governance PR: electron/governance#254
Relevant Electron PR: #22738

Feature request for splash screens: #39728

There are also more comments in this Slack thread.

I have no strong feelings about what to do with it.

@samuelmaddock
Copy link
Member

If I may, I'd like to delay merging this for maybe 3 months. I may have a potential use case for this that I'd like to try out 🙏 I certainly see the appeal of removing code which isn't utilized though so.

@nikwen
Copy link
Member
nikwen commented Apr 15, 2025

I've found people in the wild who are implementing splash screens through BrowserWindows:

https://github.com/wbosley/ElectronAppHangingOnSplashScreen/blob/bcc891d1569dd282ab9a14b6fa515fcb837790d8/src/electron/index.ts#L28-L45

(Not optimal in my opinion.)

@dsanders11
Copy link
Member

My two cents is if the code/maintenance burden is considered small enough (which I think could be argued in this case) that we should keep it and document it. I think it fills a valid use case and some apps might adopt it if it were documented.

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Apr 17, 2025
@codebytere
Copy link
Member

@ckerr output from last API meeting is we should document this as experimental, not remove it - would you rather we do that in this PR or close this and follow up with a new one?

@ckerr
Copy link
Member Author
ckerr commented Apr 22, 2025

@codebytere I'll update this PR tomorrow to mark it as experimental 👍

@ckerr
Copy link
Member Author
ckerr commented Apr 24, 2025

Actually, might be better to do it in a new PR. It doesn't make much sense for a branch named 'refactor/remove-unused-ImageView' for this task.

I'll close + delete this branch, and a stakeholder in @electron/wg-api can create a new branch to add documentation.

@ckerr ckerr closed this Apr 24, 2025
@ckerr ckerr deleted the refactor/remove-unused-ImageView branch April 24, 2025 01:33
@nikwen
Copy link
Member
nikwen commented Apr 24, 2025

Here's the follow-up issue for documenting this: #46601

@nikwen
Copy link
Member
nikwen commented Apr 24, 2025

Oh, I see we already have a new PR: #46760

Thanks, @itsananderson!

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

Successfully merging this pull request may close these issues.

5 participants
0