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

Also remove nested inclusions when removing a subtree #5720

Merged
merged 3 commits into from
Mar 29, 2024
Merged

Conversation

jleibs
Copy link
Member

@jleibs jleibs commented Mar 28, 2024

What

Introduce new helper method that not only adds an exclusion but also removes all the current matches within the subtree.

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

@jleibs jleibs marked this pull request as ready for review March 28, 2024 18:14
@jleibs jleibs added 🟦 blueprint The data that defines our UI include in changelog labels Mar 28, 2024
@@ -48,9 +47,9 @@ impl ContextMenuAction for RemoveAction {
instance_path: &InstancePath,
) {
if let Some(space_view) = ctx.viewport_blueprint.space_view(space_view_id) {
space_view.contents.add_entity_exclusion(
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be wise to also remove the old foot-gunny add_entity_exclusion, or rewrite it to never include the subtree (i.e. take an entity path instead of a EntityPathRule)

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered it, but I found the behavior that it gives in the query editor-view to still be useful.

In that view we give much clearer visibillity into all the rules that are in effect and the fact that 1 click = 1 rule modification matches what a user also does when hand-editing the expressions, where I would expect them to manually delete the inclusion lines rather than having them deleted automagically once they've typed a subtree removal expression.

@jleibs jleibs merged commit bcd2092 into main Mar 29, 2024
34 checks passed
@jleibs jleibs deleted the jleibs/fix_removals branch March 29, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🟦 blueprint The data that defines our UI include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clicking "-" to remove entity doesn't work if children were included explicitly
2 participants