Skip to content
This repository has been archived by the owner on Apr 29, 2023. It is now read-only.

Frameless Mac desktop client + improved desktop tray icon #949

Merged
merged 20 commits into from
Jul 11, 2019

Conversation

cjsauer
Copy link
Contributor

@cjsauer cjsauer commented Jul 9, 2019

To test

Close the dev tools inspector before trying these tests! Dragging the window by the nav bar does not work when this window is open!

  • Run desktop on Mac

  • Is the window frameless?

  • Is the org picker inset correctly?

  • Can you drag the window around using the nav bar?

  • Run desktop on Windows

  • Does the window still have a frame?

  • Org picker not inset?

  • Open the web app (any browser)

  • Org picker not inset?

Note: testing the new icon requires actually packaging the entire app, running locally uses the default electron icon

Very small tweaks to Windows production desktop builds.

  • Target changed to NSIS from AppX (this is much simpler)
  • Boot has some issues with paths on Windows and so there's now a new boot task with a trivial configuration difference to make it work on Windows.
  • Updated readme with Windows-specific build instructions

There aren't really any explicit test instructions for this. If you're feeling so inclined and have a Windows box laying around, please try producing a Windows production artifact locally and make sure the instructions are clear. To do this without publishing the build to GitHub Releases, use this:

boot prod-electron-windows
cd target/
yarn install
npx electron-builder --win

The resulting EXE should then be located in the dist folder.

@bago2k4
Copy link
Member

bago2k4 commented Jul 10, 2019

@cjsauer not sure the frameless is a good thing on windows, i have 2 main issues:

  • can't drag the window in any way, i can only resize it
  • the 3 window buttons in top left corner (close, expand, collapse) are in front of the app: as you also mentioned we should add a class specific to the electron container that moves the content down

Even if we solve the second the first is still a pretty bad issue, i think we should keep the frame for now.

@cjsauer
Copy link
Contributor Author

cjsauer commented Jul 10, 2019

@bago2k4 one of the test steps is to make sure Windows does have a frame, so I agree. If you’re not seeing a frame on windows then that’s a bug.

The window should be draggable by the nav bar once you’re logged in, but I do need to add dragging ability to the login screen as well.

@bago2k4
Copy link
Member

bago2k4 commented Jul 10, 2019

@cjsauer when you reply or fix a PR please remove the awaiting revision label and add back the ready for review so the reviewer will check again

@cjsauer
Copy link
Contributor Author

cjsauer commented Jul 10, 2019

@bago2k4 sure thing, wasn't quite ready for review yet. Just pushed a fix to lower the Carrot logo on the login wall for Mac builds.

NOTE: dragging the window by the nav bar does not work when the Chrome dev tools inspector is open. Make sure to close dev tools when testing!

@bago2k4
Copy link
Member

bago2k4 commented Jul 11, 2019

@cjsauer so i was able to check again on mac and the window drag works with dev tools closed and also the position of the carrot logo on login screen and of the org logo when logged in are good.

I changes the specific style are handled, added 3 classes to the body tag:

  • electron: always present on electron
  • mac-electron: for mac
  • win-electron: for win32
    and i used these classes to move the inline styles you added in the scss/partials/_electron.scss file so we can keep all the electron specific style changes here.

Check the last 2 commit and let me know if you like.

Last thing i don't have the certificates to do a staging release so i can't check the icon but i am ok merging this if you do one and i check on that.

Copy link
Member

@bago2k4 bago2k4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs your approval again @cjsauer

@cjsauer
Copy link
Contributor Author

cjsauer commented Jul 11, 2019

@bago2k4 nice, that's much better. I can pass along the certs to you.

@bago2k4 bago2k4 merged commit 1368fa2 into mainline Jul 11, 2019
@bago2k4 bago2k4 deleted the frameless-desktop branch July 11, 2019 20:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants