Add command to copy direct link to fully qualified URL#4165
Conversation
emilk
left a comment
There was a problem hiding this comment.
Nice solution! But we should be able to remove the hard-coded app.rerun.io fallback
crates/re_viewer/src/app.rs
Outdated
| self.toasts.add(toasts::Toast { | ||
| kind: toasts::ToastKind::Warning, | ||
| text: format!("Could not copy direct link, no recording is open"), | ||
| options: toasts::ToastOptions::with_ttl_in_seconds(4.0), | ||
| }); | ||
| return; |
There was a problem hiding this comment.
Should this really be an error? I would expect to just get a copy to the viewer in this case (https://app.rerun.io)
There was a problem hiding this comment.
Btw, anything logged with re_log::warn! will also show up as a warning toast
There was a problem hiding this comment.
You're right, changed it to default to location.origin by itself
crates/re_viewer/src/app.rs
Outdated
| }); | ||
| return; | ||
| }; | ||
| let direct_link = match &self.startup_options.location { |
There was a problem hiding this comment.
When is StartupOptions::location None? Looking at the code it seems to always be set on wasm, so maybe it doesn't need to be an Option? In any case, you could also call eframe::web::web_location() here if you prefer. That way we don't need the hard-coded fallback to app.rerun.io
There was a problem hiding this comment.
Looks like it's an Option because of this Default impl:
rerun/crates/re_viewer/src/app.rs
Line 70 in 2969b83
There was a problem hiding this comment.
Looks like that impl Default for StartupOptions could have a #[cfg(not(target_arch = "wasm32"))] around it though, so I don't think it needs to be an Option.
|
Changed one more thing, the command now checks if we're on |
emilk
left a comment
There was a problem hiding this comment.
Very nice to get a perma-link for free!
What
When on web, the command pallete now lists a command to copy a direct link to the "fully qualified" URL, which is
location.origin+query["url"].Fixes #4122
Checklist