Skip to content

Tray Menu Redesign #765

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 4, 2021
Merged

Tray Menu Redesign #765

merged 2 commits into from
Nov 4, 2021

Conversation

onesounds
Copy link
Contributor

#763

Other pr contents were removed and simplified.

@onesounds onesounds mentioned this pull request Oct 20, 2021
Closed
@taooceros taooceros closed this Oct 26, 2021
@taooceros taooceros reopened this Oct 26, 2021
taooceros
taooceros reviewed Oct 26, 2021
View reviewed changes
Flow.Launcher/MainWindow.xaml.cs Outdated
Comment on lines 163 to 175
var menu = contextMenu;

var header = new MenuItem() { Header = "Flow Launcher", IsEnabled = false };
var open = new MenuItem() { Header = InternationalizationManager.Instance.GetTranslation("iconTrayOpen") };
var settings = new MenuItem() { Header = InternationalizationManager.Instance.GetTranslation("iconTraySettings") };
var exit = new MenuItem() { Header = InternationalizationManager.Instance.GetTranslation("iconTrayExit") };
menu.Items[0] = header;
menu.Items[1] = open;
menu.Items[2] = settings;
menu.Items[3] = exit;
open.Click += (o, e) => Visibility = Visibility.Visible;
settings.Click += (o, e) => App.API.OpenSettingDialog();
exit.Click += (o, e) => Close();
Copy link
Member

Choose a reason for hiding this comment

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

For this part, why not keep the original one? I don't see any difference from that. Just need to change 0 to 1, 1 to 2, 2 to 3 because you have added a header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

original : ContextMenuStrip
New : Context Menu

@taooceros
Copy link
Member

Just curious, where do you change the style of the tray in this pull request?

@onesounds
Copy link
Contributor Author

onesounds commented Oct 26, 2021 •
edited
Loading

Just curious, where do you change the style of the tray in this pull request?

image
it is ContextMenuStrip. In wpf, the tray icon usually uses this as a menu.
But the styling is tricky. so people use it. https://github.com/hardcodet/wpf-notifyicon

I don't want use nuget. so I use Notifyicon + Context Menu. when Right click the notifyicon, conetextmenu will open.
but ContextmenuStirip It must exist in code. it made contextet menu automatically close when click other place. (if remove, context menu will not close when lose focus)

Where is Style?
it's "context menu". When I made it into a context menu to styling, I found out that it was already using the existing context style.
image
image

Since I already satisfied the requirements I wanted and there was a sense of unity, This is because the button size is larger and it is good to see. so I just left them as they were. I'll probably style it again later, (for dark mode) but I'll finish it like this. it's the why don't have new style.

I think UpdateNotifyIconText() (for language string update realtime) code is little bad.
I don't know how to change "var menu", so I just remove items and add again.
I think you can fix this part more beautifully. 🤤

@taooceros
Copy link
Member

I don't know how to change "var menu", so I just remove items and add again.
I think you can fix this part more beautifully. 🤤

sure

but ContextmenuStirip It must exist in code. it made contextet menu automatically close when click other place. (if remove, context menu will not close when lose focus)

That's interesting. How is that related to the new context menu?

@onesounds
Copy link
Contributor Author

That's interesting. How is that related to the new context menu?

https://stackoverflow.com/questions/1516238/how-to-get-system-tray-functionality-without-using-notifyicon-contextmenu

I don't know why exactly.

taooceros
taooceros approved these changes Oct 29, 2021
View reviewed changes
Copy link
Member

@taooceros taooceros left a comment

Choose a reason for hiding this comment

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

I will approve. Though I don't understand how the contextmenustrip works, it doesn't affect the functionality.

@taooceros taooceros requested a review from jjw24 October 29, 2021 19:53
@jjw24 jjw24 merged commit 99fcdc1 into Flow-Launcher:dev Nov 4, 2021
@jjw24 jjw24 added enhancement New feature or request kind/ui related to UI, icons, themes, etc labels Nov 4, 2021
@jjw24 jjw24 added this to the 1.9.0 milestone Nov 4, 2021
@jjw24 jjw24 mentioned this pull request Dec 5, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jjw24 jjw24 jjw24 approved these changes

@taooceros taooceros taooceros approved these changes

Assignees

@onesounds onesounds

Labels

enhancement New feature or request kind/ui related to UI, icons, themes, etc

Projects

No open projects
WIP
Status: Done

Milestone

1.9.0

Development

Successfully merging this pull request may close these issues.

3 participants

@onesounds @taooceros @jjw24