Skip to content
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

Make icon on macOS look more native #924

Merged
merged 2 commits into from
Jan 23, 2022
Merged

Conversation

ream88
Copy link
Contributor

@ream88 ream88 commented Jan 23, 2022

This PR improves the icon of the macOS desktop app to look more native.

Basically I just took the Sketch template from HIG and added static/images/logo.png to it. 😊

Here a preview on my pink wallpaper 😅

Screen Shot 2022-01-23 at 11 15 21

@CLAassistant
Copy link

CLAassistant commented Jan 23, 2022

CLA assistant check
All committers have signed the CLA.

@josevalim
Copy link
Contributor

Looks great! Can you please move the new icon to rel/app instead? The static folder is actually bundled in the package and we don't need this icon bundled. Thank you!

@ream88
Copy link
Contributor Author

ream88 commented Jan 23, 2022

Done 😊

@wojtekmach
Copy link
Contributor

wojtekmach commented Jan 23, 2022

Looks great. Note the icon is re-used as document icon.

Before:

image

After:

image

I think the former looked slightly better but it's ok to keep the latter to simplify things IMHO.

WDYT?

@josevalim
Copy link
Contributor

We can’t use different icons? Or we can but it is work?

@wojtekmach
Copy link
Contributor

We can use different icons.

@ream88
Copy link
Contributor Author

ream88 commented Jan 23, 2022 via email

@josevalim
Copy link
Contributor

Handling additional formats is a bit less straightforward, so we will merge this and open up an issue to tackle the separate files later. Thank you!

@josevalim josevalim merged commit 4bc264e into livebook-dev:main Jan 23, 2022
@josevalim
Copy link
Contributor

💚 💙 💜 💛 ❤️

@ream88 ream88 deleted the mac-icon branch January 24, 2022 14:24
@wojtekmach
Copy link
Contributor

@ream88 blast from the past :) I didn't realise that if we configure document types with the icon, the icon is taken as is, which means we lose the automatic "livemd" text on the icon, etc.

Per https://developer.apple.com/design/human-interface-guidelines/foundations/icons#document-icons there should be a document type template in HIG . If you could create an icon per template that has the Livebook logo we'd really appreciate it! We basically want this:

image

@ream88
Copy link
Contributor Author

ream88 commented Nov 21, 2022

I will look into that!

@ream88
Copy link
Contributor Author

ream88 commented Nov 21, 2022

I'm currently experiencing crashes like these mentioned here: #1363

@josevalim
Copy link
Contributor

What is in your logs?

@ream88
Copy link
Contributor Author

ream88 commented Nov 21, 2022

[LivebookLauncher] release pid: 2779
[LivebookLauncher] release pid: 3279
[LivebookLauncher] release pid: 3803
[LivebookLauncher] release pid: 4644
[LivebookLauncher] release pid: 4838
[LivebookLauncher] release pid: 4972

Basically the same @elepedus was encountering.

These are my locally used versions of Elixir and Erlang:

elixir          1.14.0          /Users/mario/.tool-versions
elm             ______          No version is set. Run "asdf <global|shell|local> elm <version>"
erlang          25.1.2          /Users/mario/.tool-versions
nodejs          16.4.2          /Users/mario/.tool-versions
python          3.9.1           /Users/mario/.tool-versions
ruby            2.7.2           /Users/mario/.tool-versions

@wojtekmach
Copy link
Contributor

How are you installing desktop? Are you downloading from https://github.com/livebook-dev/livebook#desktop-app ?

@ream88
Copy link
Contributor Author

ream88 commented Nov 21, 2022

Yes

@wojtekmach
Copy link
Contributor

If you open Console.app and Crash Reports, is there anything for Livebook?

@ream88
Copy link
Contributor Author

ream88 commented Nov 21, 2022

Not really, but I guess my issues are out of the scope of this issue 😉. For the icon I just created a dummy Xcode app and associated a custom document icon to it:

Screenshot 2022-11-21 at 18 16 47

Now the question is: Should we use "LIVEBOOK" or "LIVEMD"?

@wojtekmach
Copy link
Contributor

I think it should be livemd but didn’t check HIG yet

@ream88
Copy link
Contributor Author

ream88 commented Nov 21, 2022

The HIG state the following:

Icon Text (optional): Input the text you want displayed on the bottom of the document icon. This can be the same as the extension or something more descriptive. For example, the scn extension can use scene as its text.

https://developer.apple.com/news/?id=5i6jlf4d

@wojtekmach
Copy link
Contributor

Oh, got it, thanks. Up to you!

@wojtekmach
Copy link
Contributor

About the crash, any chance you're on an ARM Mac?

@ream88
Copy link
Contributor Author

ream88 commented Nov 22, 2022

About the crash, any chance you're on an ARM Mac?

Yes on a M1 Mac.

@ream88
Copy link
Contributor Author

ream88 commented Nov 22, 2022

Also how can I build the app locally? Would like to build to test if the icons are properly working 😊

@wojtekmach
Copy link
Contributor

wojtekmach commented Nov 22, 2022

One more question, if you right-click on the app and "Get Info", is "Run in Rosetta" (or something similar) checked? It definitely shouldn't. I sometimes saw it checked and never figured why it was so, but I think there is a way to fix it.

To build it locally you can run sh .github/scripts/app/build_mac.sh

@ream88
Copy link
Contributor Author

ream88 commented Nov 22, 2022

Omg @wojtekmach, that fixed it! Not sure why the checkbox was checked though.. Maybe we should add this somewhere in the README.

@wojtekmach
Copy link
Contributor

Great, thank you for verifying this. No idea either. I think there's a setting we can add to Info.plist to force the correct architecture so I'll investigate. Thanks again!

@wojtekmach wojtekmach mentioned this pull request Nov 24, 2022
@wojtekmach
Copy link
Contributor

@ream88 ping. :) We can definitely go with "LIVEBOOK" text on the icon like you proposed. Totally up to you!

@ream88
Copy link
Contributor Author

ream88 commented Dec 10, 2022

Sorry for the silence … I had some problems getting the icons to work on my Mac. Will try to finish it this weekend!

ream88 added a commit to ream88/livebook that referenced this pull request Dec 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants