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.

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

Collection of Menu alterations #230

Merged
merged 14 commits into from
Apr 14, 2024
Merged

Collection of Menu alterations #230

merged 14 commits into from
Apr 14, 2024

Conversation

trigg
Copy link
Contributor

@trigg trigg commented Apr 11, 2024

Options added:

  • Application icons in list form panel/menu_list
  • Menu shows application categories panel/menu_show_categories
  • Categories width panel/menu_min_category_width

New Menu Features:

  • Categories to the left of application list
  • Keyboard navigation of applications & categories
  • Right click an application to choose between alternative actions
  • Changing search term selects first item in list
  • Categories hidden if no applications exist for it

Commits include work done for a different PR merged here and then reverted as the patch was unrelated.

trigg added 7 commits April 5, 2024 08:34
@trigg
- Check for and load absolute path icons
21cae62
@trigg
- Moved theme value after first potential return
0e2334e 
- Cleaned up some style
@trigg
Add Categories to panel
3ea04e7
@trigg
menu: Add DesktopEntry actions menu
9a9ee15 
menu: Catch and inject typing on popover
menu: Always select first item in menu when typing in search
menu: Always reset to "All" Category on open
menu: Add optional list view
menu: Use gtk ellipsize to shorten labels
@trigg
Revert "- Moved theme value after first potential return"
8166de1 
This reverts commit 0e2334e.
@trigg
Revert "- Check for and load absolute path icons"
d51de00 
This reverts commit 21cae62.
@trigg
- Added example config
5409ff1
trigg
trigg commented Apr 11, 2024
View reviewed changes
src/panel/widgets/menu.cpp Outdated Show resolved Hide resolved
trigg
trigg commented Apr 11, 2024
View reviewed changes
src/panel/widgets/menu.cpp
category_definitions["System"] = std::make_unique<WfMenuCategoryDefinition>("System", "applications-system");
category_definitions["Utility"] = std::make_unique<WfMenuCategoryDefinition>("Accessories", "applications-utilities");
category_definitions["Hidden"] = std::make_unique<WfMenuCategoryDefinition>("Other Desktops", "user-desktop");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These decide which of the XDG desktop entry spec get used for our categories.

Removing one means it gets omitted, adding another means we gain a new potential category.

I think we should remove "Hidden" as it will fill with settings managers for unrelated desktops, but opinions may vary so I've kept it until we decide

Copy link
Member

Choose a reason for hiding this comment

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

I do think it makes sense to have one fallback where everything not fitting other categories is present. I'm certain some apps will be mis-labeled, so this way the user can still see them ..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Prefer safe to sorry, I see that.

Then potentially a bool to hide the category?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And further : Should 'Other desktop' always be included in 'All' and in searching?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should hide any applications from the user, then again, I usually don't use the menu, so if you feel strongly in either direction, I'd trust your intuition.

ammen99
ammen99 reviewed Apr 12, 2024
View reviewed changes
src/panel/widgets/menu.hpp Outdated Show resolved Hide resolved
ammen99
ammen99 approved these changes Apr 14, 2024
View reviewed changes
Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

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

In general LGTM, let me know if you want to make any more changes, otherwise, I will merge.

Also a note, I usually squash all commits together to avoid style fix commits in the history, if you want your work to be remain in multiple commits, please squash manually. Otherwise I'll just squash and you don't have to do anything.

@trigg
Copy link
Contributor Author

trigg commented Apr 14, 2024

... otherwise, I will merge.

That's fine. While I'm unsure everyone will agree with me about the defaults, they're certainly acceptable defaults.

... Otherwise I'll just squash and you don't have to do anything.

Go for it, I'm not precious about it.

@ammen99
Copy link
Member

ammen99 commented Apr 14, 2024

Alright, I think even if someone disagrees, we can add more options in follow-up PRs, you did the harder part ;) Thanks!

@ammen99 ammen99 merged commit a8fc045 into WayfireWM:master Apr 14, 2024
2 checks passed
@trigg trigg deleted the menu-testing branch April 14, 2024 12:26
This was referenced Apr 16, 2024
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ammen99 ammen99 ammen99 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@trigg @ammen99