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

More context menu 2: add support to show/hide DataResults #5397

Merged
merged 3 commits into from
Mar 6, 2024

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Mar 5, 2024

What

The Hide and Show actions now support DataResults as well.

With that, it's easier than ever to run into:

image

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!

@abey79 abey79 added ui concerns graphical user interface include in changelog labels Mar 5, 2024
Base automatically changed from antoine/mcm1-refactor to main March 5, 2024 11:50
@abey79 abey79 force-pushed the antoine/mcm2-dataresult-visibility branch from 0c81247 to 244b37c Compare March 5, 2024 11:54
@abey79 abey79 changed the title More context menu 2: DataResult visibility More context menu 2: add support to show/hide DataResults Mar 5, 2024
@teh-cmc teh-cmc self-requested a review March 6, 2024 08:49
Copy link
Member

@teh-cmc teh-cmc left a comment

Choose a reason for hiding this comment

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

Those checks are getting out of control 😬

Copy link
Member

Choose a reason for hiding this comment

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

This check does way too much imo; it needs to be broken down into smaller, atomic checks.

At the very least each of these sub-sections should be its own check, though I'm sure it can be made even more fine-grained than that.

I should be able to go through a check with my brain turned off. I shouldn't have to keep track of my progress or some mental state, and if the check fails I shouldn't have to explain where and how, because it should be small and focused enough that it doesn't matter.

Copy link
Member Author

Choose a reason for hiding this comment

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

Noted. I'll do that as a follow-up PR 👍🏻

crates/re_viewport/src/context_menu/actions.rs Outdated Show resolved Hide resolved
@abey79 abey79 merged commit e77b518 into main Mar 6, 2024
34 checks passed
@abey79 abey79 deleted the antoine/mcm2-dataresult-visibility branch March 6, 2024 10:35
@abey79 abey79 mentioned this pull request Mar 11, 2024
5 tasks
abey79 added a commit that referenced this pull request Mar 12, 2024
### What

NOTE: review commit by commit (first commit is pure refactoring).

Final PR in the series. This PR:
- split `actions.rs` into many sub-files
- fix capitalisation in some menu item
- split the release check list in small checks

Full series:
- #5392
- #5397
- #5407
- #5411
- #5422
- #5433
- #5456

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5456/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5456/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5456/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/5456)
- [Docs
preview](https://rerun.io/preview/a36360f586494f6648fdc4bbb9d806ab12911358/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/a36360f586494f6648fdc4bbb9d806ab12911358/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include in changelog ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants