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

[feat] Automatically set app name in About menu item #571

Closed
caesar opened this issue Sep 25, 2022 · 13 comments
Closed

[feat] Automatically set app name in About menu item #571

caesar opened this issue Sep 25, 2022 · 13 comments

Comments

@caesar
Copy link
Contributor

caesar commented Sep 25, 2022

For consistency, it would be nice to implement this in a similar way to PR #570 (issue #536).

I didn't consider implementing it in that PR because of the following considerations:

  • It would be a breaking change, involving removal of the first field of the MenuItem::About(String, AboutMetadata) enum variant.

  • It might require implementation for every platform – I am unsure of platform conventions for Linux and Windows so this would require some investigation.

I am happy to investigate and implement this if the breaking change is approved.

@amrbashir
Copy link
Member

Other platforms don't have a reliable way to get the app name, that's why we need it tb explicity specified

@caesar
Copy link
Contributor Author

caesar commented Sep 26, 2022

Hmm, that sucks. But do other platforms even need the name? I don't know about Linux, but so far as I'm aware on Windows the platform convention is Help > About with no app name.

edit: That said, it's a while since I've used Windows much, so maybe that's changed in recent years?

@caesar
Copy link
Contributor Author

caesar commented Sep 26, 2022

so far as I'm aware on Windows the platform convention is Help > About with no app name

For example, the Microsoft design guide here shows the following example:

    <muxc:MenuBarItem Title="Help">
        <MenuFlyoutItem Text="About"/>
    </muxc:MenuBarItem>

@caesar
Copy link
Contributor Author

caesar commented Sep 26, 2022

On Linux, maybe we could use glib::application_name?
(Assuming the platform convention, to the extent that there is such a thing, is to show the name.)

@amrbashir
Copy link
Member

Yes other platforms do only show "About" in the menu item but the name along with othe metadata are shown in the dialog that appears when it is clicked.

As for glib::application_name, it will only return a name if there was a previous call to set_application_name oor set_prgname

@caesar
Copy link
Contributor Author

caesar commented Sep 26, 2022

the name along with othe metadata are shown in the dialog that appears when it is clicked

I see. Would including it in AboutMetadata be an option? Seems like a more logical place, if possible (again, obviously a breaking change though).

On a side note I'd love it if AboutMetadata was optional… since in my apps I have separate menubar implementations for each platform and only the Linux one needs it…

As for glib::application_name, it will only return a name if there was a previous call to set_application_name oor set_prgname

Sounds like something Tao or Tauri should be doing anyway… isn't it used anywhere else?

@amrbashir
Copy link
Member

the name along with othe metadata are shown in the dialog that appears when it is clicked

I see. Would including it in AboutMetadata be an option? Seems like a more logical place, if possible (again, obviously a breaking change though).

AboutMetadata is ignored on macOS, that's why we kept app_name separate. Also app_name is required at least on Windows and Linux so it makes sense to also require it on macOS.

On a side note I'd love it if AboutMetadata was optional… since in my apps I have separate menubar implementations for each platform and only the Linux one needs it…

Every single field of AboutMetadata is optional, so it does make sense to make it optional all together, it also implements Default trait so you can also just do AboutMetadata::default()

As for glib::application_name, it will only return a name if there was a previous call to set_application_name oor set_prgname

Sounds like something Tao or Tauri should be doing anyway… isn't it used anywhere else?

Not really, tao shouldn't care about it and even if tauri wants to, we never use glib::application_name anywhere so I wouldn't recommend doing in tauri as well.

@caesar
Copy link
Contributor Author

caesar commented Sep 26, 2022

AboutMetadata is ignored on macOS, that's why we kept app_name separate.

Well, that's the point of this issue, on macOS app_name should also be ignored… 🙂

Also app_name is required at least on Windows and Linux so it makes sense to also require it on macOS.

I don't agree… 🙂 but tbh it's only a small annoyance to me so if you prefer to keep it that way I'm happy to close this.

we never use glib::application_name anywhere

What I meant was that I would assume there are places where the OS displays the application name. But I'm not really familiar with how these things work on Linux so maybe glib::application_name isn't used for that.

@caesar
Copy link
Contributor Author

caesar commented Sep 26, 2022

app_name is required at least on Windows and Linux

Is MenuItem::About actually even implemented on Windows? The docs say no, and I can't find an implementation...

In which case we're only talking about Linux needing the app name, which in my opinion would mean it was much cleaner if it was in the AboutMetadata.

But again if you feel this is too trivial to bother with I don't mind 😄

@amrbashir
Copy link
Member

AboutMetadata is ignored on macOS, that's why we kept app_name separate.

Well, that's the point of this issue, on macOS app_name should also be ignored… slightly_smiling_face

Also app_name is required at least on Windows and Linux so it makes sense to also require it on macOS.

I don't agree… slightly_smiling_face but tbh it's only a small annoyance to me so if you prefer to keep it that way I'm happy to close this.

Just because we can deduce the app_name on macOS, doesn't mean we should mark it as optional. Since the majority requires it, we make macOS comply with the rest. After all this is a cross-platform solution so we have to make trade-offs.

we never use glib::application_name anywhere

What I meant was that I would assume there are places where the OS displays the application name. But I'm not really familiar with how these things work on Linux so maybe glib::application_name isn't used for that.

application_name is used internally for gtk, the OS doesn't read it AFAIK.

@caesar
Copy link
Contributor Author

caesar commented Sep 26, 2022

Since the majority requires it, we make macOS comply with the rest

I don't feel Linux is "the majority", and I suggested a cleaner solution by merging it with the extra metadata that's already required for Linux.

But never mind, there are bigger issues to worry about, I'll close this.

@caesar caesar closed this as completed Sep 26, 2022
@amrbashir
Copy link
Member

I don't feel Linux is "the majority",

The majority is not Linux only, The majority is Windows and Linux. tao's menu system doesn't support About menu item on Windows but I added support for it in my rewrite in https://github.com/amrbashir/muda

I suggested a cleaner solution by merging it with the extra metadata that's already required for Linux.

That is not true, AboutMetadata is not required at all, every field is optional and it will only show the fields you specify, so you can use AboutMetadata::default and nothing will render except the App name.

@caesar
Copy link
Contributor Author

caesar commented Sep 26, 2022

The majority is not Linux only, The majority is Windows and Linux. tao's menu system doesn't support About menu item on Windows but I added support for it in my rewrite in https://github.com/amrbashir/muda

Fair point, it could be implemented for Windows too – though since you also use the AboutMetadata in your implementation, I feel the point is somewhat moot…

That is not true, AboutMetadata is not required at all, every field is optional and it will only show the fields you specify, so you can use AboutMetadata::default and nothing will render except the App name.

That's true. It feels like an edge case, but I guess if you wanted an about box with only a name, that would make the call slightly cleaner.

Anyway enough bikeshedding, the current implementation works, I'll put my objections aside and work on something that actually matters 😉

@caesar caesar closed this as not planned Won't fix, can't repro, duplicate, stale Sep 26, 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

No branches or pull requests

2 participants