Page MenuHomePhabricator

Add custom behavior to the CellTruncationProvider

Authored by prha on Aug 14 2019, 10:03 PM.


Group Reviewers
Restricted Project
R1:d79b27377be1: Add custom behavior to the CellTruncationProvider

The CellTruncationProvider could only show raw text in a dialog
because it only had access to generated DOM, instead of react elements. This
diff adds an event handler to the CellTruncationProvider to allow for customized
expansion behavior (when a user clicks on 'View Full Message').

This diff pushes the CellTruncationProvider down into a new <LogsRow />
component, which has branching behavior based on the event node type. The
original structured content rendering logic was extracted into a separate
component (LogsRowStructuredContent), so we can render the exact same content
in the truncated table view as well as the unabridged modal view.

CellTruncationProvider becomes a little more generic (no longer tied to the
structure of the rendered DOM) at the expense of forcing each event type to
consider the truncated view flow.

Related Issue:

Test Plan

Rendered Unstructured, Failure, Materialization events, resized window,
triggered the 'View Full Message' link.

Diff Detail

R1 dagster
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

prha created this revision.Aug 14 2019, 10:03 PM
prha edited the summary of this revision. (Show Details)Aug 14 2019, 10:08 PM
prha added reviewers: Restricted Project, bengotow.

this looks good to me but ill give @bengotow time to look


want to fix this (use ref) while you're here?

prha added inline comments.Aug 15 2019, 4:01 PM

Yeah, I spent maybe 45 min yesterday trying to get ref working here, transferring it to the child prop, but couldn't figure it out.

I should at least leave the original github issue comment here.

prha updated this revision to Diff 3726.Aug 15 2019, 4:30 PM
  • Use refs instead of ReactDOM.findDOMNode
prha added a comment.Aug 15 2019, 4:32 PM

ended up just wrapping the content with a wrapper div... I don't think there's another way to do it

bengotow accepted this revision.Aug 15 2019, 6:47 PM

This looks great! This approach definitely seems more flexible.

This revision is now accepted and ready to land.Aug 15 2019, 6:47 PM
This revision was automatically updated to reflect the committed changes.