Improve space view tab design #2879
Conversation
|
Two open questions TBD @martenbjork
|
|
Slack huddle summary:
|
abey79
changed the title
I'll make a quick, separate PR with that change, so that we can look at the result and possibly iterate. |
crates/re_viewport/src/viewport.rs
Outdated
| let space_view = self | ||
| .space_views | ||
| .get(space_view_id) | ||
| .expect("Should have been populated beforehand"); |
There was a problem hiding this comment.
let's not crash here. We need to be super careful in the ui with any unwrap/expect calls and avoid them almost always in any viewer code.
This one for instance might easily happen with some frame delayed data - i.e. the space view got destroyed but the tabs didn't as just itself yet. Imho we should just silently ignore this case and render nothing (putting an error_once feels like the right thing, but then this may just spurious pop up with some users when it's probably not an issue after all)
There was a problem hiding this comment.
Guilty of copy/paste :) This applies the same to fn pane_ui() a few lines above, right? I'll change both.
There was a problem hiding this comment.
OK fixed. I cheated in tab_ui() and delegated to the status quo, which debug_once!(). Which btw makes me think that this situation must not occur that often otherwise we'd have seen that debug output.
There was a problem hiding this comment.
I don't think debug log is printed by default
crates/re_viewport/src/viewport.rs
Outdated
| let icon = space_view.class(self.ctx.space_view_class_registry).icon(); | ||
| let image = self.ctx.re_ui.icon_image(icon); | ||
| let texture_id = image.texture_id(ui.ctx()); | ||
| let text_to_icon_padding = 4.0; |
There was a problem hiding this comment.
didn't we want to make this a constant on re_ui? guess that's a follow-up
There was a problem hiding this comment.
Right. This can become a constant. It'd be an improvement even against the status quo.
What
This PR improves the space view tab UI:
The following changes will be subject to a follow-up PR:
ReUi::selectable_label_with_icon()for the implementation (if it makes sense after (1))Closes #2737
New design:
Checklist