Make the Data panel show type-driven widgets for attributes#4062
Make the Data panel show type-driven widgets for attributes#4062
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the data panel's navigation system by introducing a PathStep enum to support drilling down into both elements and attributes. It updates the TableRowLayout trait and implements a dispatch mechanism for various attribute types. Feedback was provided to optimize the identifier method for String by reducing the number of iterations over its lines.
| } | ||
| } | ||
| fn cell_widget(&self, _target: PathStep) -> WidgetInstance { | ||
| TextLabel::new(self.identifier()).narrow(true).widget_instance() |
There was a problem hiding this comment.
The identifier() function for String is called here. Its current implementation iterates over the string's lines twice to determine if there is more than one line, which can be inefficient for very long strings.
You can make this more efficient by using a single iterator and only checking for the existence of a second line, like this:
fn identifier(&self) -> String {
// Show the first line, and if there are more, indicate that with an ellipsis
let mut lines = self.lines();
let first_line = lines.next().unwrap_or("");
if lines.next().is_some() {
format!("\"{} …\"", first_line)
} else {
format!("\"{}\"", first_line)
}
}This avoids a second full iteration over the string's lines.
c079df5 to
9818eda
Compare
Instead of the MVP attributes implementation's text label driven strings for showing attribute values, now it shows an actual widget per attribute value type. Partly closes #3779.