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

Persist servers and allow adding/removing them from the UI #9086

Merged
merged 17 commits into from
Feb 21, 2025

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Feb 19, 2025

What

With this PR, the servers are now persisted across viewer relaunch in the Redap browser, and the can be added and removed from the UI. The handling of the collection query management has been improved, so was its UI.

Note: this makes a pre-existing crossbeam-channel dependency explicit.

Add/remove buttons:

image



Add server modal:

image

@abey79 abey79 added ui concerns graphical user interface exclude from changelog PRs with this won't show up in CHANGELOG.md feat-redap-browser Everything related to the in-viewer Redap browser include in changelog and removed exclude from changelog PRs with this won't show up in CHANGELOG.md labels Feb 19, 2025
# Conflicts:
#	crates/viewer/re_redap_browser/src/servers.rs
Copy link

github-actions bot commented Feb 19, 2025

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link Manifest
bec44ad https://rerun.io/viewer/pr/9086 +nightly +main

Note: This comment is updated whenever you push a commit.

# Conflicts:
#	Cargo.lock
#	crates/top/rerun/src/commands/entrypoint.rs
#	crates/utils/re_uri/src/lib.rs
#	crates/viewer/re_redap_browser/src/collection_ui.rs
#	crates/viewer/re_redap_browser/src/servers.rs
#	crates/viewer/re_viewer/src/app.rs
@abey79 abey79 marked this pull request as ready for review February 20, 2025 11:28
@@ -183,6 +183,7 @@ const_format = "0.2"
convert_case = "0.6"
criterion = "0.5"
crossbeam = "0.8"
crossbeam-channel = "0.5"
Copy link
Member Author

Choose a reason for hiding this comment

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

was already in the dependency tree

Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

didn't venture too deep into the actual ui code, but data flow turned out nicely now I think :)

use re_viewer_context::{AsyncRuntimeHandle, WasmNotSend};

/// A handle to an object that is requested asynchronously.
pub enum RequestedObject<T: Send + 'static> {
Copy link
Member

Choose a reason for hiding this comment

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

this turned out really nice and short :)

pub fn ui(&self, ctx: &ViewerContext<'_>, ui: &mut egui::Ui) {
//TODO(ab): we should display something even if no catalog is currently selected.
fn add_server_modal_ui(&mut self, ui: &egui::Ui) {
//TODO(ab): borrow checker doesn't let me use `with_ctx()` here, I should find a better way
Copy link
Member

Choose a reason for hiding this comment

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

ah that pattern is tricky, don't know either. Oooooonnneeee day the Polonius borrow checker will solve this, any decade now

Copy link
Member Author

Choose a reason for hiding this comment

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

in fairness, pulling that off would be pretty great!

@abey79 abey79 merged commit e363ea4 into main Feb 21, 2025
35 checks passed
@abey79 abey79 deleted the antoine/add-remove-servers branch February 21, 2025 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat-redap-browser Everything related to the in-viewer Redap browser include in changelog ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants