-
Notifications
You must be signed in to change notification settings - Fork 104
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
dexc-desktop: Use macdrive for Darwin #2372
Conversation
Changes included in 638752b Support for multiple windows (max: 5. this is not really a requirement just keeping things sane?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When? I know some CSS styling is not supported on Webkit. Even some "default" styling in other browsers could also be missing. EDIT: I'm unable to reproduce, tried building the CSS files again? : scroll.mp4EDIT 2: I notice the white bg on hover. @martonp what did you expect? |
Rebased.. |
Seems OK now.. maybe it's an intermittent issue or I didn't build it properly. But the build script builds the UI every time.. |
Yup, that was why I did this #2333 (comment) earlier since I'd have to build several times to test everything was fine when working on that PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We never shut down on Mac?
Window is not opening for me after installation. Logs show that dexc is shutting down right away.
...
2023-06-09 22:18:47.663 [INF] CORE: Connected to 1 of 1 DEX servers
2023-06-09 22:18:47.663 [INF] CORE: Loaded btc wallet configuration.
2023-06-09 22:18:47.663 [INF] CORE: Loaded dcr wallet configuration.
2023-06-09 22:18:47.663 [INF] CORE: Loaded 0 active orders.
2023-06-09 22:18:47.663 [INF] CORE: Loaded 0 active match orders
2023-06-09 22:18:47.663 [DBG] CORE: starting fiat rate fetching
2023-06-09 22:18:47.663 [DBG] WEB: Using embedded site resources.
2023-06-09 22:18:47.663 [INF] WEB: Using language en-US
2023-06-09 22:18:47.663 [INF] WEB: Using embedded HTML templates
2023-06-09 22:18:47.679 [DBG] APP: Exiting dexc main.
Weird. I wonder what could be terminating dexc-desktop instance on your machine. |
If you mean automatically(i.e. after all windows have been closed and no active order), no. The user will have to shut down dexc-desktop on macOS using the |
4ecd799
to
b05889f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm glad, you were able to test this PR and it's running fine on your end. I will look into the issue with the links. |
In the "Window" section of the top toolbar, there are options for tabs, i.e. "Show Tab Bar", "Show Previous Tab". Is it possible to get rid of those? |
Yeah, it is possible. I looked into it over the weekend. |
Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
- Add support for upto 5 windows for macOS. - Add icon to system status bar. - Add more webview config preferences. - Refactor. Signed-off-by: Philemon Ukane <[email protected]>
- Handle defer statements properly for darwin. - Remove syncserver for darwin - Refactor - Prevent multiple dexc-desktop instance. Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
Rebased for 36a9a1a. |
Signed-off-by: Philemon Ukane <[email protected]>
This looks A-OK to me. I'm unlikely to get a Mac to test anytime soon, so please decide when it's good to merge @buck54321 and pull the trigger when you like. |
I am yet to push changes relating to the recent reviews but the major changes I'd like to add here are:
|
Uploads? For the wallet config file and dex cert file selections? Do they not work? |
Yes, they won't work unless we implement a special method for that. |
Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
Signed-off-by: Philemon Ukane <[email protected]>
@chappjc, @buck54321, @martonp, blockers mentioned in #2372 (comment) have been addressed in my recent commits(i.e 3f1052d). |
Signed-off-by: Philemon Ukane <[email protected]>
- Implement self-signed cert challenge handler for MacOS app Signed-off-by: Philemon Ukane <[email protected]>
d53fd37
to
d161e66
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @ukane-philemon. I've confirmed that links and file uploads work.
I noticed that when I open the app, the window opens but doesn't take focus. Anybody else seeing this? I thought it was working before.
I've just looked into this and I don't know how we can make the login form take focus automatically in |
After poking around I found out we can achieve that behaviour with But IMO, I think we should enable this behaviour. Saves users the extra burden of interacting with the app. After all, they opened it and intend to use it. What do you think @buck54321? |
Builds on #2333.
I researched other options and MacDrive seem to be a reasonable option. It allows using supported native Mac APIs, which covers our use case.
I was able to implement some "native" macOS app behaviour and features; keeping the app running without windows, creating new windows, and having a dock icon menu.
I'm putting this up so we can have discussions around it and clarify what feature is missing (menu items etc)
EDIT: If we want to keep the New Window menu as @martonp mentioned here #2333 (comment), we'd have to allow more than one window on macOS.