Improve UI of various components in the selection panel #6297
Conversation
abey79
changed the title
UiLayout::List of many DataUi implementationDataUi implementation compliant with the constraints of UiLayout::List
abey79
changed the title
DataUi implementation compliant with the constraints of UiLayout::ListDataUi implementations compliant with the constraints of UiLayout::List
abey79
force-pushed
the
antoine/data-ui-list-update
branch
from
2a9e780
to
5f871e3
Compare
|
before: |
Wumpf
changed the title
DataUi implementations compliant with the constraints of UiLayout::List
crates/re_data_ui/src/data.rs
Outdated
| impl DataUi for Range1D { | ||
| fn data_ui( | ||
| &self, | ||
| _ctx: &ViewerContext<'_>, | ||
| ui: &mut Ui, | ||
| ui_layout: UiLayout, | ||
| _query: &LatestAtQuery, | ||
| _db: &EntityDb, | ||
| ) { | ||
| label_for_ui_layout(ui, ui_layout, self.to_string()); | ||
| } | ||
| } | ||
|
|
||
| impl DataUi for Range2D { | ||
| fn data_ui( | ||
| &self, | ||
| _ctx: &ViewerContext<'_>, | ||
| ui: &mut Ui, | ||
| ui_layout: UiLayout, | ||
| _query: &LatestAtQuery, | ||
| _db: &EntityDb, | ||
| ) { | ||
| label_for_ui_layout(ui, ui_layout, self.to_string()); | ||
| } | ||
| } |
There was a problem hiding this comment.
so before we had data ui implement for those we'd just generate a label automatically. Shouldn't that label generation use label_for_ui_layout instead?
There was a problem hiding this comment.
Are you referring to text_ui() (in component_ui_registry.rs)? I believe several code path lead to that function.
Looking at it a bit more... It's essentially doing a more fancy version of label_for_ui_layout, so these are possibly duplicates. text_ui uses monospace font though, I'm not sure why. Seems like some further cleaning is in order around here.
There was a problem hiding this comment.
See update PR description. There is much work to be done here still.
crates/re_data_ui/src/image.rs
Outdated
| shapes | ||
| } | ||
| format!( | ||
| "shape: {shapes}{}", |
There was a problem hiding this comment.
the shape: part isn't useful and just takes space imho. Interestingly, on the screenshots you took one has it and one doesn't ;-)
(I checked locally, it's always there now)
There was a problem hiding this comment.
yep, one screenshot slept through outdated :)
That said: I dont think 4, 3, 5, 1 by itself is very descriptive. that looks very much like a 1x4 int tensor (which it is not).
I guess I could go 4x3x5x1, but still need to special case when dimension have labels. Will give it a second look.
There was a problem hiding this comment.
I figured out a decent way, see updated screenshots. I also added some cases to the checklist.
abey79
changed the title
…aces instead of `label_for_ui_layout`
What
UiVerbositytoUiLayoutand make its variants more precise #6291UiVerbositytoUiLayoutand tighten up related implementations #6245ListItemstyle for the component list (in the Selection Panel when an entity is selected) #4161This PR fixes the
UiLayout::Listimplementations ofDataUisuch that they all fit on a single line and are deal with potentially narrow space (mostly via truncation).This PR unearthed quite some inconsistencies in how we're using monospace font for data. For now,
text_ui(which uses monospace) has been renameddata_label_for_ui_layoutto parallel the newlabel_for_ui_layout(which uses proportional). In the future (#6315), we must unify both with a better, flexible API that also allows mixed styles.TODO:
size(shape) > 3Screenshots
When truncated:
Checklist
mainbuild: rerun.io/viewernightlybuild: rerun.io/viewerTo run all checks from
main, comment on the PR with@rerun-bot full-check.