Skip to content

Make the Data panel show type-driven widgets for attributes#4062

Merged
Keavon merged 1 commit intomasterfrom
data-panel-type-attribute-widgets
Apr 28, 2026
Merged

Make the Data panel show type-driven widgets for attributes#4062
Keavon merged 1 commit intomasterfrom
data-panel-type-attribute-widgets

Conversation

@Keavon
Copy link
Copy Markdown
Member

@Keavon Keavon commented Apr 28, 2026

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.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 4 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

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()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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.

@Keavon Keavon force-pushed the data-panel-type-attribute-widgets branch from c079df5 to 9818eda Compare April 28, 2026 10:38
@Keavon Keavon merged commit 7f48e17 into master Apr 28, 2026
9 of 10 checks passed
@Keavon Keavon deleted the data-panel-type-attribute-widgets branch April 28, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Data trees

1 participant