Universal Visibility Action (#2320 cont'd) #2426
Conversation
`action_toggle_visible` -> `action_toggle_visibility` Matches with `EVENT_TOGGLE_VISIBILITY`
Codecov Report
@@ Coverage Diff @@
## master #2426 +/- ##
======================================
Coverage 9.83% 9.83%
======================================
Files 147 147
Lines 10492 10489 -3
======================================
Hits 1032 1032
+ Misses 9460 9457 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
IPC commands are no longer necessary now that the actions are implemented. Changed some method permissions as well to reflect this.
|
Nice 😃 I don't really have time to review right now, I got a lot going on. It's gonna be at least a few weeks until I get to this. |
patrick96
left a comment
There was a problem hiding this comment.
Looks good!
This change does also require documentation in doc/user/actions.rst. I would say we add a new subsection to "Available Actions" named "All modules" at the beginning, just before the date module.
Also, maybe we should, just by convention, have a module_ prefix for all global module actions to prevent future naming conflicts.
To be consistent with the command names to change bar visibility, we could use the names module_toggle, module_show, module_hide. What do you think?
include/modules/meta/base.inl
Outdated
|
|
||
| template <typename Impl> | ||
| void module<Impl>::set_visible(bool value) { | ||
| // m_log.info("%s: Visibility changed (state=%s)", m_name, value ? "shown" : "hidden"); |
There was a problem hiding this comment.
| // m_log.info("%s: Visibility changed (state=%s)", m_name, value ? "shown" : "hidden"); | |
| m_log.info("%s: Visibility changed (state=%s)", m_name, value ? "shown" : "hidden"); |
Yeah, that sounds like a good idea. I remember not being sure what to call them. I'll get the changes up in a few minutes. |
- `module_toggle` - `module_show` - `module_hide` Delineate common actions to all modules with a `module_` prefix (for future actions too)
What type of PR is this? (check all applicable)
Description
Adds universal module visibility control actions
toggle_visible,set_visible, andset_invisible.Related Issues & Documents
#2108
#2320
#2342
Documentation (check all applicable)