Ylianst / MeshCentral

A complete web-based remote monitoring and management web site. Once setup you can install agents and perform remote desktop session to devices on the local network or over the Internet.
https://meshcentral.com
Apache License 2.0
4.27k stars 572 forks source link

Defining elements styles in CSS rather than handlebars templates #147

Closed elastalink closed 5 years ago

elastalink commented 5 years ago

I would like to modernize UI through custom style.css, but most of the elements styles are embedded within handlebars templates. Therefore, any changes we made in templates are overwritten on a new release.

If you are interested, we would like to contribute to your project and work on moving styles to style.css and updating *.handlebars files while maintaining the same level of functionality and design.

Before we issue PR, let me know if you are open to this change.

Ylianst commented 5 years ago

Yes! This would be great. If you can start with small pull requests to make me comfortable with the changes. It's possible you may retrain me on how to do CSS correctly. Help with fixing CSS would be very much appreciated.

elastalink commented 5 years ago

No problem. We plan first to move styles from HTML part of the template to styles.css. Most of your elements already have id's, so its just matter of defining the styles. And then for JS part of the template used to change DOM, instead of making changes per element (using QS functions), we would add class to the body (for example "fullscreen") which will affect all the elements using CSS selector defined in style.css. If you are OK with this strategy, how about if we start with login.handlebars as a proof of concept?

Ylianst commented 5 years ago

Sounds great and yes, starting with the login screen is a great way to start. Many thanks.

elastalink commented 5 years ago

No problem and THANK YOU for this great project. Here is our first request: #159

Ylianst commented 5 years ago

Can we close this issue? I think we can comment when PR's are submitted. However, I am not a GitHub expert.

elastalink commented 5 years ago

I suggest to leave it open as a centralized reference to upcoming PR, until all the styles are moved from templates to CSS. But if you prefer to close it, that is OK as well.

elastalink commented 5 years ago

Our 2nd pull request is focused on default.handlebars template and extending style.css: #170

Ylianst commented 5 years ago

So, I accepted the request however like the first pull request, it's clearly never been tested at all and breaks a lot of things. Now, I click on a device and I see a gray screen. I guess I have to spend today fixing all of the issues.

elastalink commented 5 years ago

Hmm, we can not replicate this situation on our forked branch. Since you merged our PR, we will clone your master and check where is the issue. Can we have a quick call before we both start making changes? I will send you my contact info to your hotmail address.

Ylianst commented 5 years ago

Thanks for chatting, your help is very much appreciated. I will hold off on changing default.handlebars and style.css for a while until your team takes a look at it. Just "npm install meshcentral" and put the two latest files (default.handlebars / style.css) in the server and take a look, the problems should be obvious. Thank you in advance.

elastalink commented 5 years ago

Great, we are on it.

Ylianst commented 5 years ago

Great. Published MeshCentral v0.3.1-z with all the CSS changes.

Ylianst commented 5 years ago

Arg. I just noticed a gap at the bottom on the login screen. Working on fixing that.

elastalink commented 5 years ago

Tnx. We are experimenting with a new approach using CSS grid, which is ideal for a fullscreen layouts and it will take care of hight adjustments currently done is JS. But there are challenges combining CSS grid and a centered layout with fixed width, on the same page! Give us few days to tryout some ideas...

Ylianst commented 5 years ago

Replacing any JS with CCS would be great, I do notice it in the rendering speed. Can't wait to see what you guys come up with.

elastalink commented 5 years ago

3rd PR #189 : All styles from default.handlebars are now moved to CSS. For FullScreen layout, we added CSS grid that will handle content resizing to fit available height. And we added two new toggle buttons (icons):

  1. Changing main menu position (left bar vs horizontal) on a FullScreen layout view. So if you connect to a device with a widescreen, your desktop will be a bit larger.
  2. Changing the desktop view aspect ratio: proportional to resolution vs stretch to fit the available area. Works in all 3 view modes (centered, full screen, or full desktop) and has a smart scaling on window resizing.

We will pause working on other templates and allow time for testing.

Ylianst commented 5 years ago

Hi. Thanks for the work, I saw the new aspect radio button, I like it! I took a quick look and as with previous PR's it's not well tested and a bunch of things are broken.

Let me know if you want me to fix all these. Again, thanks for all the changes, very appreciated. Ylian

e1 e2 e3 e4 e5

Ylianst commented 5 years ago

Looking into it a bit more:

e6 e7 e8 e9

elastalink commented 5 years ago

Sorry about that, we expected a few issues, but not so many. Thank you for fast feedback and please continue listing all the issues on this thread. We are committed to getting them all fixed and resubmit PR.

We already fixed the rest of the reported issues except IE support and the My Devices list not filling the list (2nd column being empty on your last screenshot). We can not recreate this condition and I was hoping if you can let us know how this happened (above you also have a screenshot of My Devices screen that properly devided list in two columns).

BTW: I didn't realize that you are still trying to support IE11, since Edge is for a while default Windows browser and IE has limited support for WebRTC and CSS Grid. But if want, we can try to make CSS grid work with IE.

elastalink commented 5 years ago

All reported issues are now fixed in PR #193, except we could not recreate the issue with a list of devices not aligned to fill the screen. On our tests, it works on all browsers even when you hit refresh without need to call deskAdjust() twice.

Thank you for being patient and if you find any more bugs we will be happy to fix them.

Ylianst commented 5 years ago

Perfect, thanks! Let me give it a try!

Ylianst commented 5 years ago

Not sure what is going on, but the UI is now unusable, picture below. Maybe I am not doing something right? I did get the latest login.hb, default.ng and style.css... however, it's clearly not right at all. If it works correctly on your side, let me know.

PR193-1 PR193-2

elastalink commented 5 years ago

Something must be wrong with GitHub merge. After you merged PR #193, I cloned the master and files were not merged correctly. I sent 3 files to your private email. Please try to overwrite them on your clean clone and if you are still getting strange UI issues, please call me. You can see a 9min video of a quick walkthrough of latest CSS with IE, Edge, FireFox and Chrome: https://youtu.be/N8bKEET4YEQ

Ylianst commented 5 years ago

I will give it another try later today. If the styles.css was not correct, that would cause the issues I have seen.

Ylianst commented 5 years ago

I just took a look at the 3 files you mailed me and yes, they work great! I could not find any issues. This is great work. I love it. One thing I do want to fix is the little UI switch arrows so that existing users will see the same UI as they are used to. I have lots of users and I don't want the UI to change on them unless they change it. I will work on that tomorrow and get it released.

FYI. If your company is offering a service with MeshCentral and Intel vPro, let me know. I will gladly promote it.

elastalink commented 5 years ago

Great! Do you know what got broken after merging PR? Hopefully, in the future, there will be fewer changes and PR will work properly. Do you want us to continue working on other handlebars view files?

BTW, thank you for offering to promote us. We are hoping to launch after summer a PC-as-a-service package based on Intel NUC's with vPro and remote management based on MeshCentral/MeshCommander (and now with IDER - amazing !). This might be a good case study for you. I'll keep you posted...

jsastriawan commented 5 years ago

Hi Elastalink,

Your contribution to the project is amazing. I believe we can help you on white paper etc.

Regards,

Joko

Ylianst commented 5 years ago

Published MeshCentral v0.3.2-t with the new CSS. I made some fixes and changed the way we switch between UI modes. I also added a full screen button to the Intel AMT tab. It looks good and I like the new CSS.

elastalink commented 5 years ago

Thank you @jsastriawan. We are glad to be involved in such a great project!

Ylianst commented 5 years ago

Arg. It looks like the new CSS is completely messed up on MacOS Safari. Could you take a look at it? Let me know if not, I will if needed.

MeshCentral-Safari

elastalink commented 5 years ago

@Ylianst, we tested with Safari on two different MAC's (macos sierra and mojave) and everything looks perfect. I don't know what is causing this issue on your MAC. Since we cannot replicate, can you look at it?

Screen Shot 2019-04-23 at 10 52 38 PM
Ylianst commented 5 years ago

I am going to close this issue as the CSS in MeshCentral as been updated and works ok. I still have problems with older browsers that I may fix at some point. Don't hesitate to open a new issue if we can cooperate on more things.