Make it sexier in macOS #336
Conversation
|
|
|
What happens if you hide the server list? |
|
@edipox This looks good, but the only issue is that if you hide the server list it looks like this: Are you able to make it so that when you toggle the server list you get the original title bar back? |
|
@alexbrazier thank you! Setting the title bar back is a good idea too. From my side I have another idea for possible ways to fix this. But I couldn't go back to it yet |
|
@alexbrazier I tried to add the title bar back but I couldn't. I don't know if that is possible without re creating the window. So the solution I came with is... kind of a hack: injecting css in the webview content to fix the issue. It's an ugly hack but it works
|
|
Awesome @edipox , looks good. I think you are right that you have to inject css, but I found a function to do it without having to execute javascript using |
src/scripts/sidebar.js
Outdated
| @@ -131,6 +132,11 @@ class SideBar extends EventEmitter { | |||
| img.src = `${hostUrl}/assets/favicon.svg?v=${Math.round(Math.random()*10000)}`; | |||
| } | |||
|
|
|||
| fixTitleBar () { | |||
| const fixScript = 'document.body.innerHTML = document.body.innerHTML + "<style>.side-nav{ margin-top: 15px; }</style>"'; | |||
| document.getElementsByTagName('webview')[0].executeJavaScript(fixScript); | |||
There was a problem hiding this comment.
This will always add the margin even if you show the side bar. It will also only work on the first server.
Try moving the CSS injection to the show and hide functions like this in hide:
document.querySelectorAll('webview').forEach((webviewObj) => webviewObj.insertCSS('aside.side-nav{margin-top:15px;overflow:hidden;}'));
And this in show:
document.querySelectorAll('webview').forEach((webviewObj) => webviewObj.insertCSS('aside.side-nav{margin-top:0;overflow:auto;}'));
You could also create an injectCSS function in the webview class to reduce this duplicated code
There was a problem hiding this comment.
Awesome suggestions! Thanks @alexbrazier I'll go for that later
|
Hi @alexbrazier I followed your suggestions. I had to create some functions because I needed to do the fix for the first render. I also added an animation for the server list hide event. |
|
Fixed conflicts with develop |
src/scripts/sidebar.js
Outdated
| @@ -192,12 +192,20 @@ class SideBar extends EventEmitter { | |||
| document.body.classList.add('hide-server-list'); | |||
| localStorage.setItem('sidebar-closed', 'true'); | |||
| this.emit('hide'); | |||
| if (process.platform === 'darwin') { | |||
| [].forEach.call(document.getElementsByTagName('webview'), | |||
There was a problem hiding this comment.
If you use querySelectorAll('webview') you can use forEach like document.querySelectorAll('webview').forEach(... instead of using [].forEach
There was a problem hiding this comment.
The first time I tried I got an error that's why a went getElementsByTagName, but I guess the error was related to something else because it's working now 👍
|
Hi @rodrigok I couldn't reproduce the color diff bug.
As you can see in the gif, initially I was able to drag the window, but when I open the dev tools I lost the ability to drag the window as I normally do, this seems to be a known electron issue given that another user experienced it here: electron/electron#3329 (comment) In the final part of the gif you can see that I was still able to drag the window with dev tools open (inner layout) by using the resize windows controls. Here you can find another example of this: electron/electron#3329 (comment) |
|
@rodrigok For me colour difference only seems to exist after you change the colour. If you reload the server, or application then it picks up the correct colour. Is it the same for you? |
|
@alexbrazier Yeap, you'r right. @edipox Yes, the DD works if I close the dev tools |
|
@engelgabriel What do you think? Can we merge this? |
@RocketChat/core
Changes to make it look cooler in macOS by removing the titlebar