Skip to content

[Control Group]: Option to toggle floating actions#153019

Closed
logeekal wants to merge 1 commit intoelastic:mainfrom
logeekal:feat/floating_actions_toggle
Closed

[Control Group]: Option to toggle floating actions#153019
logeekal wants to merge 1 commit intoelastic:mainfrom
logeekal:feat/floating_actions_toggle

Conversation

@logeekal
Copy link
Copy Markdown
Contributor

@logeekal logeekal commented Mar 9, 2023

Summary

This PR adds the option to toggle floating actions.

Motivation

In security solution, we have a need to disable floating actions selectively in some cases. For example, user can delete the controls but we want to enforce at least one control on the screen. Hence, we want to allow users to edit the control but not delete it.

Screen.Recording.2023-03-09.at.15.18.36.mov

@logeekal logeekal added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// release_note:skip Skip the PR/issue when compiling release notes labels Mar 9, 2023
@logeekal logeekal requested a review from a team as a code owner March 9, 2023 14:23
@logeekal logeekal changed the title feat: toggle floating actions [Control Group]: Option to toggle floating actions Mar 9, 2023
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
controls 268 270 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
controls 179.4KB 179.6KB +226.0B
Unknown metric groups

API count

id before after diff
controls 272 274 +2

ESLint disabled line counts

id before after diff
securitySolution 432 434 +2

Total ESLint disabled count

id before after diff
securitySolution 509 511 +2

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@Heenawter
Copy link
Copy Markdown
Contributor

Heenawter commented Mar 9, 2023

This is definitely an important feature! I am actually in the process of moving the hover actions to the uiActions framework. The PR is still in draft, but I am currently registering the actions as part of the ControlGroup container - I wonder if, however, we should instead move this registration out to the consumer (so for the Presentation use case, we would register the Control hover actions alongside the other Dashboard actions).

This would definitely help in situations where, for example, the consumer only wants to give users the ability to edit controls, and not delete them - they just wouldn't register the delete hover action in their use case. To support the concept of conditional disabling, such as the "only disable delete when there is one control remaining" idea, we would have to some generic logic to the isCompatible method that would allow the parents of the control group (i.e. the Dashboard or some other consumer) - I would need to think this through more.

Just throwing some ideas out there - @ThomThomson interested in your thoughts 👀 This probably warrants a larger discussion, haha!

@ThomThomson
Copy link
Copy Markdown
Contributor

@Heenawter, I think that the plugin ui actions are registered in has no bearing on where they show up. In any case, this use case of conditionally disabling actions on an embeddable is already covered by the disabledActions input key.

@logeekal is it possible to wait for this feature until Hannah's PR for using uiActions is complete? We'll need to ensure to provide an API method to set disabled actions, and we'll also need to make sure that the control frame can respond to changes in that state on the fly, but overall we can probably work that functionality into the PR on our side.

If you need this feature sooner than that, could you potentially use the existing disabledActions for this rather than creating a new key on the input disabledFloatingActions?

@Heenawter
Copy link
Copy Markdown
Contributor

@logeekal #153065 has been merged, so you should now be able to control which actions are available via the disabledActions property of the control group's explicit input 🎉

I added an example of this to the example plugin, but let me know if you have any other questions/concerns about this approach - hopefully that works much better for ya'll :)

@logeekal
Copy link
Copy Markdown
Contributor Author

This is awesome news @Heenawter . Thank you very much. I will try it out and will get back to you.

@logeekal logeekal closed this Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t//

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants