SpacingBat3
Custom Window Bar
~~Also, I may not be able to merge PRs that don't sign their commits:~~
Update: no longer the requirement. I can bypass checks and sign commits on my own when it is merged.
@goosesima please resolve all of the conversations (if you disagree with my suggestions, just reply to me why and mark the conversation as resolved) and ESLint warnings. You can see them at Actions summary page. As of the ESLint errors, I will try to help you on that – I believe I have the write access to this repo, maybe I'll resolve a few things both on upstream and help you to resolve some issues with this fork and there won't be no need to restructure the code greatly in order to comply with my demands.
Meanwhile, the testcase builds are available as artifacts at: https://github.com/SpacingBat3/WebCord/actions/runs/2912335992
See new commit.
Looks like a lot of great changes, actually! I see you've separated the CSS from the one that is always injected, which I approve. You've made a lot of changes great and I'm getting more happy to merge this PR.
I still need to find out if Discord has an access to the titlebar and if so, I would prefer to leave this as a note like it was before and move it to advanced section.
I saw you were actually hiding titlebar when it is loading. I don't know what to say about that, other than I've expected it would be preserved there. But maybe I'll accept it after I'll review everything properly and it will satisfy all my requirements.
To be honest, even after that review I'm happy with the changes you've recently done, your code now just needs to be a little cleared up to pass the WebCord's quality checks and preferably follow some of my suggestions (or reply to my concerns). I'm amazed you've actually using the BrowserView and althrough using the CSS to add a margin sounds like a bad workaround and maybe the better would be to actually load Discord in the BrowserView as well (maybe only Discord? I think then taskbar could be just added as a part of the window and DevTools shortcut could be added to the BrowserView instead...), but that's I agree that can be outside of this PR's scope and I'm OK with the current implementation. I might probably work on it myself or make an use of my write access to this repo and actually help you dealing with that as well as fixing some issues that was part of the review.
FYI, I might be working right now with PulseAudio and try do something useful with that Node module I've recently fixed [unix: connections], maybe do some other contributions on that project. It seems to be used as a dependency of balenalabs/balena-sound which looks like to be quite popular, although I have to admit I've never looked for a project like that and I'm unsure if I ever heard about the project like this. I might use it in WebCord for screen sharing audio recording, which looks like to be one of the features most demanded by the community and not so hopeless considering there's already a dirty workaround-like implementation of audio sharing in WebCord.
FYI
I don't think implementation of audio sharing in WebCord is good idea. Because it need implemented in chromium. Chromium can record audio from tab, on Windows window's audio also. But on Linux there not implemented. This will help for Element client and other web apps, and not just Discord.
FYI
I don't think implementation of audio sharing in WebCord is good idea. Because it need implemented in chromium. Chromium can record audio from tab, on Windows window's audio also. But on Linux there not implemented. This will help for Element client and other web apps, and not just Discord.
I think its a great idea as its annoying not having audio on my screensharing. Sure it maybe isn't the final solution but it sure is a step in the correct direction.
@goosesima looks like the title bar interferes with DevTools and has some flaws like dragable load page or inaccesible buttons before full page initialization (it should work immediately from the user perspective), I think I'll just move it to the Advanced section before merging unless you still want to polish it.
I may also work on the better design from it on CSS side. I just don't like some design choices like height of the title bar and I would like to improve it to match my personal preference or maybe compare it with Discord to see how different it is. I don't promise I will change anything, but I will experiment with redesigning it and see if that is better or not IMO.
@SpacingBat3
interferes with DevTools
Yes, it hide width and height of window (this shows when user resize window). Fix: Use for Discord BrowserView, not BrowserWindow and offset it by Y (BrowserView). Does BrowserView support Chrome extensions?
some flaws like dragable load page
I made it to look like in Windows client Discord.
https://github.com/SpacingBat3/WebCord/blob/efce472e630b4abe2a033a57113b36f69f186794/sources/assets/web/css/load.css#L11
When Discord loads you can drag it.
inaccesible buttons before full page initialization
I didn't know this. I think this possible fix with switch to BrowserView.
some design choices like height
I'm not sure, but height 20-22px on Windows official client.
Can you merge and move it to the Advanced section?
Yeah, I'll eventually do that. I still didn't approve this through as I want to play with its design a little. As long as I have the write access you don't need to adapt it, I'll handle this on my own. You can take a break on working on it, I'll now test it, maybe do some fixes to make it look fine on all platforms. You made a great job!
Also in reply to this:
When Discord loads you can drag it.
WebCord's design differs here a little, since I use a single window for loading. There was some job to actually make a splash screen look like in Discord, but I want to be a little original rather than clone everything straight from Discord. IMO WebCord should look similar, but it could differ in some ways. WebCord also don't need a splash on its own and I plan to split it from the Discord's. So...
Yes, it hide width and height of window (this shows when user resize window)
For me (on Linux) it also interferes with the tabs shown on top of the DevTools page.
Fix: Use for Discord BrowserView, not BrowserWindow and offset it by Y (BrowserView).
Moving to BrowserView is what I've considered to do, but because of the bug I've encountered, I gave up on it for some time. But now I know the cause of it, so I think I may work on it again.
This would also mean that the title bar could also be added directly to the BrowserWindow, so it would work and be shown immediately after the window shows.
Does BrowserView support Chrome extensions?
I think Chrome extensions are specific to webContents, and BrowserView (which can be treated as a separate page/tab in browser) contains its own webContents. So yea, that should work...
(...) I made it to look like in Windows client Discord. (...) I'm not sure, but height 20-22px on Windows official client.
I'm also thinking for this reason that I might limit it for Windows-only, as many users on macOS and Linux might not be comfortable with it (on Linux, Windows uses non-native title bar and on macOS the title bar might include the traffic lights and be partially native). I might also improve the look of WebCord on Windows as I've noticed some API that might be useful for that
@goosesima I'll probably fix issues in your branch once I'll have a time for that and possibly merge it to community, where I'll keep some partially-finished features before they land to master.