Summary

Add default renderers for group actions.

Review Request #14700 — Created Nov. 18, 2025 and submitted Dec. 10, 2025, 1:10 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-7.1.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

These can now specify the default renderer for any group actions. With
this, an attachment point for a menubar could specify that all group
actions are rendered as menus, and groups in sidebars are rendered as
subsections, as a couple of examples.

As an internal optimization, actions now have an is_action_group
boolean property, which differentiates between an action and an action
group. This avoids the more costly isinstance checks that'd otherwise
need to be done multiple times per action render.

I also noted cases where fallback renderers weren't being used, and
where an detailed menu item action failed to render correctly with an
action lacking an icon. These have been fixed.

Testing Done

Unit tests pass.

Tested that existing actions work as expected.

Tested along with the upcoming addition of sidebar renderers.

Commits

Summary	ID
Add default renderers for group actions. Attachment points and groups both support specifying default renderers for any actions nested within. This applied to both standard actions and group actions, which would do the wrong thing for group actions, and necessitated that group actions always specify their own renderers. These can now specify the default renderer for any group actions. With this, an attachment point for a menubar could specify that all group actions are rendered as menus, and groups in sidebars are rendered as subsections, as a couple of examples. Group renderers can assign a special value of "self" for subgroup renderers, which ensures the same group renderer is used for its children. Group renderers are also now responsible for rendering their children, moving much of the logic from `{% child_actions_html %}` into a new `BaseActionGroupRenderer.render_children()` method. As an internal optimization, actions now have an `_is_action_group` boolean property, which differentiates between an action and an action group. This avoids the more costly `isinstance` checks that'd otherwise need to be done multiple times per action render. I also noted cases where fallback renderers weren't being used, and where an detailed menu item action failed to render correctly with an action lacking an icon. These have been fixed.	6c921413768587ed07e84713f9c062a466fa70d6

Summary

Add default renderers for group actions.

Attachment points and groups both support specifying default renderers for any actions nested within. This applied to both standard actions and group actions, which would do the wrong thing for group actions, and necessitated that group actions always specify their own renderers. These can now specify the default renderer for any group actions. With this, an attachment point for a menubar could specify that all group actions are rendered as menus, and groups in sidebars are rendered as subsections, as a couple of examples. Group renderers can assign a special value of "self" for subgroup renderers, which ensures the same group renderer is used for its children. Group renderers are also now responsible for rendering their children, moving much of the logic from `{% child_actions_html %}` into a new `BaseActionGroupRenderer.render_children()` method. As an internal optimization, actions now have an `_is_action_group` boolean property, which differentiates between an action and an action group. This avoids the more costly `isinstance` checks that'd otherwise need to be done multiple times per action render. I also noted cases where fallback renderers weren't being used, and where an detailed menu item action failed to render correctly with an action lacking an icon. These have been fixed.

6c921413768587ed07e84713f9c062a466fa70d6

Issues

Description	From	Last Updated
Just noticed that this class docstring is missing "Version Added"	david	Nov. 20, 2025, 11:51 a.m.
Given that this is not public API, should we call it _is_action_group?	david	Nov. 20, 2025, 12:30 p.m.
Can we add the "Instance variables" comment in here?	david	Nov. 20, 2025, 12:30 p.m.
'reviewboard.actions.renderers.ActionSubgroupRendererType' imported but unused Column: 5 Error code: F401	reviewbot	Nov. 28, 2025, 5:02 p.m.
line too long (89 > 79 characters) Column: 80 Error code: E501	reviewbot	Nov. 28, 2025, 5:02 p.m.
line too long (95 > 79 characters) Column: 80 Error code: E501	reviewbot	Nov. 28, 2025, 5:02 p.m.
continuation line under-indented for visual indent Column: 39 Error code: E128	reviewbot	Nov. 28, 2025, 5:03 p.m.
'logging' imported but unused Column: 1 Error code: F401	reviewbot	Nov. 28, 2025, 5:03 p.m.
Can we add a comment here explaining this case, something like "This attachment point does not allow for child actions."	maubin	Dec. 9, 2025, 11:50 p.m.
This is missing its docstring.	maubin	Dec. 9, 2025, 11:53 p.m.
Seems like these should probably both be logger.error. I don't think we need the traceback in the second case.	david	Dec. 1, 2025, 3:25 p.m.

flake8 passed.

JSHint passed.

reviewboard/actions/base.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Just noticed that this class docstring is missing "Version Added"
```

reviewboard/actions/base.py (Diff revision 1)

The issue has been resolved. Show all issues

Given that this is not public API, should we call it _is_action_group?

reviewboard/actions/renderers.py (Diff revision 1)

The issue has been dropped. Show all issues

Can we add the "Instance variables" comment in here?

chipx86 Nov. 28, 2025, 5:02 p.m.

These are all class variables. I'll type them as such. Initially I had intended to let these be set as instance variables, but I've changed that design.

Change Summary:


Added BaseActionGroupRenderer.render_children() to handle children rendering, recursively. This addresses issues with renderer selection from the previous code, and can recurse.
Added support for 'self' as a value for subgroup renderers, so a group can easily recurse. The template tag is now a thin wrapper around this.
Renamed is_action_group to _is_action_group.
default_item_renderer_cls and default_subgroup_renderer_cls are now ClassVars.
Added a missing Version Added.

Commits:

	Summary	ID
	Add default renderers for group actions. Attachment points and groups both support specifying default renderers for any actions nested within. This applied to both standard actions and group actions, which would do the wrong thing for group actions, and necessitated that group actions always specify their own renderers. These can now specify the default renderer for any group actions. With this, an attachment point for a menubar could specify that all group actions are rendered as menus, and groups in sidebars are rendered as subsections, as a couple of examples. As an internal optimization, actions now have an `is_action_group` boolean property, which differentiates between an action and an action group. This avoids the more costly `isinstance` checks that'd otherwise need to be done multiple times per action render. I also noted cases where fallback renderers weren't being used, and where an detailed menu item action failed to render correctly with an action lacking an icon. These have been fixed.	7a279edeaf0730e8c028399fa73ffe61c3f927d3
	Add default renderers for group actions. Attachment points and groups both support specifying default renderers for any actions nested within. This applied to both standard actions and group actions, which would do the wrong thing for group actions, and necessitated that group actions always specify their own renderers. These can now specify the default renderer for any group actions. With this, an attachment point for a menubar could specify that all group actions are rendered as menus, and groups in sidebars are rendered as subsections, as a couple of examples. Group renderers can assign a special value of "self" for subgroup renderers, which ensures the same group renderer is used for its children. Group renderers are also now responsible for rendering their children, moving much of the logic from `{% child_actions_html %}` into a new `BaseActionGroupRenderer.render_children()` method. As an internal optimization, actions now have an `_is_action_group` boolean property, which differentiates between an action and an action group. This avoids the more costly `isinstance` checks that'd otherwise need to be done multiple times per action render. I also noted cases where fallback renderers weren't being used, and where an detailed menu item action failed to render correctly with an action lacking an icon. These have been fixed.	24cb0636eac8536774cd08e3ec2378e49e02a9b4

Diff:

Revision 2 (+1236 -168)

Show changes

	reviewboard/actions/base.py
	reviewboard/actions/renderers.py
	reviewboard/actions/templatetags/actions.py
	reviewboard/actions/tests/base.py
	reviewboard/actions/tests/test_action_attachment_point.py
	reviewboard/actions/tests/test_base_action_group_renderer.py
	reviewboard/actions/tests/test_default_action_group_renderer.py
	reviewboard/actions/tests/test_group_action.py
	1 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/actions/base.py (Diff revision 2)

The issue has been resolved. Show all issues

'reviewboard.actions.renderers.ActionSubgroupRendererType' imported but unused

Column: 5
Error code: F401

reviewboard/actions/tests/test_base_action_group_renderer.py (Diff revision 2)
The issue has been resolved. Show all issues
```
line too long (89 > 79 characters)

Column: 80
Error code: E501
```
reviewboard/actions/tests/test_base_action_group_renderer.py (Diff revision 2)
The issue has been resolved. Show all issues
```
line too long (95 > 79 characters)

Column: 80
Error code: E501
```
reviewboard/actions/tests/test_base_action_group_renderer.py (Diff revision 2)
The issue has been resolved. Show all issues
```
continuation line under-indented for visual indent

Column: 39
Error code: E128
```
reviewboard/actions/tests/test_default_action_group_renderer.py (Diff revision 2)
The issue has been resolved. Show all issues
```
'logging' imported but unused

Column: 1
Error code: F401
```

Change Summary:


Removed unused imports.
Fixed wrapping issues.

Commits:

	Summary	ID
	Add default renderers for group actions. Attachment points and groups both support specifying default renderers for any actions nested within. This applied to both standard actions and group actions, which would do the wrong thing for group actions, and necessitated that group actions always specify their own renderers. These can now specify the default renderer for any group actions. With this, an attachment point for a menubar could specify that all group actions are rendered as menus, and groups in sidebars are rendered as subsections, as a couple of examples. Group renderers can assign a special value of "self" for subgroup renderers, which ensures the same group renderer is used for its children. Group renderers are also now responsible for rendering their children, moving much of the logic from `{% child_actions_html %}` into a new `BaseActionGroupRenderer.render_children()` method. As an internal optimization, actions now have an `_is_action_group` boolean property, which differentiates between an action and an action group. This avoids the more costly `isinstance` checks that'd otherwise need to be done multiple times per action render. I also noted cases where fallback renderers weren't being used, and where an detailed menu item action failed to render correctly with an action lacking an icon. These have been fixed.	24cb0636eac8536774cd08e3ec2378e49e02a9b4
	Add default renderers for group actions. Attachment points and groups both support specifying default renderers for any actions nested within. This applied to both standard actions and group actions, which would do the wrong thing for group actions, and necessitated that group actions always specify their own renderers. These can now specify the default renderer for any group actions. With this, an attachment point for a menubar could specify that all group actions are rendered as menus, and groups in sidebars are rendered as subsections, as a couple of examples. Group renderers can assign a special value of "self" for subgroup renderers, which ensures the same group renderer is used for its children. Group renderers are also now responsible for rendering their children, moving much of the logic from `{% child_actions_html %}` into a new `BaseActionGroupRenderer.render_children()` method. As an internal optimization, actions now have an `_is_action_group` boolean property, which differentiates between an action and an action group. This avoids the more costly `isinstance` checks that'd otherwise need to be done multiple times per action render. I also noted cases where fallback renderers weren't being used, and where an detailed menu item action failed to render correctly with an action lacking an icon. These have been fixed.	482b22fedfcf47078a5f68d167cb8225eea3e1f5

Diff:

Revision 3 (+1234 -168)

Show changes

	reviewboard/actions/base.py
	reviewboard/actions/renderers.py
	reviewboard/actions/templatetags/actions.py
	reviewboard/actions/tests/base.py
	reviewboard/actions/tests/test_action_attachment_point.py
	reviewboard/actions/tests/test_base_action_group_renderer.py
	reviewboard/actions/tests/test_default_action_group_renderer.py
	reviewboard/actions/tests/test_group_action.py
	1 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

reviewboard/actions/renderers.py (Diff revision 3)

The issue has been resolved. Show all issues

Seems like these should probably both be logger.error. I don't think we need the traceback in the second case.

chipx86 Dec. 1, 2025, 3:25 p.m.
```
Not sure what happened there.
```

reviewboard/actions/base.py (Diff revision 3)

The issue has been resolved. Show all issues

Can we add a comment here explaining this case, something like "This attachment point does not allow for child actions."

reviewboard/actions/renderers.py (Diff revision 3)
The issue has been resolved. Show all issues
```
This is missing its docstring.
```

Change Summary:


Added a comment explaining what occurs when rendering children using a renderer that isn't a group renderer.
Added a missing docstring.

Commits:

	Summary	ID
	Add default renderers for group actions. Attachment points and groups both support specifying default renderers for any actions nested within. This applied to both standard actions and group actions, which would do the wrong thing for group actions, and necessitated that group actions always specify their own renderers. These can now specify the default renderer for any group actions. With this, an attachment point for a menubar could specify that all group actions are rendered as menus, and groups in sidebars are rendered as subsections, as a couple of examples. Group renderers can assign a special value of "self" for subgroup renderers, which ensures the same group renderer is used for its children. Group renderers are also now responsible for rendering their children, moving much of the logic from `{% child_actions_html %}` into a new `BaseActionGroupRenderer.render_children()` method. As an internal optimization, actions now have an `_is_action_group` boolean property, which differentiates between an action and an action group. This avoids the more costly `isinstance` checks that'd otherwise need to be done multiple times per action render. I also noted cases where fallback renderers weren't being used, and where an detailed menu item action failed to render correctly with an action lacking an icon. These have been fixed.	482b22fedfcf47078a5f68d167cb8225eea3e1f5
	Add default renderers for group actions. Attachment points and groups both support specifying default renderers for any actions nested within. This applied to both standard actions and group actions, which would do the wrong thing for group actions, and necessitated that group actions always specify their own renderers. These can now specify the default renderer for any group actions. With this, an attachment point for a menubar could specify that all group actions are rendered as menus, and groups in sidebars are rendered as subsections, as a couple of examples. Group renderers can assign a special value of "self" for subgroup renderers, which ensures the same group renderer is used for its children. Group renderers are also now responsible for rendering their children, moving much of the logic from `{% child_actions_html %}` into a new `BaseActionGroupRenderer.render_children()` method. As an internal optimization, actions now have an `_is_action_group` boolean property, which differentiates between an action and an action group. This avoids the more costly `isinstance` checks that'd otherwise need to be done multiple times per action render. I also noted cases where fallback renderers weren't being used, and where an detailed menu item action failed to render correctly with an action lacking an icon. These have been fixed.	6c921413768587ed07e84713f9c062a466fa70d6

Diff:

Revision 4 (+1292 -168)

Show changes

	reviewboard/actions/base.py
	reviewboard/actions/renderers.py
	reviewboard/actions/templatetags/actions.py
	reviewboard/actions/tests/base.py
	reviewboard/actions/tests/test_action_attachment_point.py
	reviewboard/actions/tests/test_base_action_group_renderer.py
	reviewboard/actions/tests/test_default_action_group_renderer.py
	reviewboard/actions/tests/test_group_action.py
	1 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-7.1.x (1f1f6e8)