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

Allow extra_menu: tag for nested menus. #4830

Merged
merged 3 commits into from
Dec 21, 2022
Merged

Allow extra_menu: tag for nested menus. #4830

merged 3 commits into from
Dec 21, 2022

Conversation

portnov
Copy link
Collaborator

@portnov portnov commented Dec 20, 2022

This allows to process extra_menu: tags for nested menus in index.yaml.

Preflight checklist

Put an x letter in each brackets when you're done this item:

  • Code changes complete.
  • Code documentation complete.
  • Documentation for users complete (or not required, if user never sees these changes).
  • Manual testing done.
  • Unit-tests implemented.
  • Ready for merge.

@portnov portnov requested a review from Durman December 20, 2022 14:26
Comment on lines 520 to 526
for cat in add_node_menu:
if getattr(cat, 'extra_menu', '') == self.menu_name:
cat.draw(_self.layout)
elif isinstance(cat, Category):
for item in cat:
if getattr(item, 'extra_menu', '') == self.menu_name:
item.draw(_self.layout)
Copy link
Collaborator

@Durman Durman Dec 20, 2022

Choose a reason for hiding this comment

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

I guess this method can be used instead

def walk_categories(self) -> 'Category':

Suggested change
for cat in add_node_menu:
if getattr(cat, 'extra_menu', '') == self.menu_name:
cat.draw(_self.layout)
elif isinstance(cat, Category):
for item in cat:
if getattr(item, 'extra_menu', '') == self.menu_name:
item.draw(_self.layout)
for cat in add_node_menu.walk_categories():
if getattr(cat, 'extra_menu', '') == self.menu_name:
cat.draw(_self.layout)

Probably I preferred the first variant for performance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, updated.
I had to add a parameter to walk_categories to include CustomMenu items in order to include Groups and Presets into 5 menu.

Copy link
Collaborator

@Durman Durman Dec 20, 2022

Choose a reason for hiding this comment

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

I did not realize that Custom Menus are also needed. Then using walk_categories it not best choice. The idea was that the method returns only categories. The annontation also show this. And there is recursive call which makes things a bit more complicated. I can suggest than:

sub_categories = [add_node_menu]
for _ in range(1000):
    cat = sub_categories.pop()
    for cat_item in cat:
        if getattr(cat, 'extra_menu', '') == self.menu_name:
            cat_item.draw(_self.layout)
        if hasattr(cat, '__iter__'):
            sub_categories.append(cat_item)
    if not sub_categories:
        break
else:
    log('There is some problem')

@portnov portnov merged commit 172c8de into master Dec 21, 2022
@portnov portnov deleted the nested_extra_menus branch December 21, 2022 15:45
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.

2 participants