Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: fix expander bug #677

Merged
merged 2 commits into from
Feb 2, 2023

Conversation

ursucarina
Copy link
Contributor

Signed-off-by: Carina Ursu [email protected]

Read then delete this section

- Make sure to use a concise title for the pull-request.

- Use #patch, #minor or #major in the pull-request title to bump the corresponding version. Otherwise, the patch version
will be bumped. More details

TL;DR

Please replace this text with a description of what this PR accomplishes.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

Remove the 'fixes' keyword if there will be multiple PRs to fix the linked issue

fixes https://github.com/flyteorg/flyte/issues/

Follow-up issue

NA
OR
https://github.com/flyteorg/flyte/issues/

Signed-off-by: Carina Ursu <[email protected]>
@ursucarina ursucarina requested review from a team, eugenejahn, jsonporter and olga-union and removed request for a team February 2, 2023 20:39
Comment on lines 70 to 83
const expanderContent = React.useMemo(() => {
return isParentNode(nodeExecution) ? (
<RowExpander
ref={ref as React.ForwardedRef<HTMLButtonElement>}
key={node.scopedId}
expanded={isExpanded(node)}
onClick={() => {
onToggle(node.id, node.scopedId, nodeLevel);
}}
/>
) : (
<div className={styles.leaf} />
);
}, [node, nodeLevel]);
Copy link
Contributor

Choose a reason for hiding this comment

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

@ursucarina we need to duplicate this in timeline tab (TaskNames.tsx), or maybe move it to a tiny component which we can use here and in TaskNames

Signed-off-by: Carina Ursu <[email protected]>
Copy link
Contributor

@olga-union olga-union left a comment

Choose a reason for hiding this comment

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

🥇 🥇 🥇

@ursucarina ursucarina merged commit 6a48658 into olga-union/execution-view-refactor Feb 2, 2023
@ursucarina ursucarina deleted the carina/expander branch February 2, 2023 22:56
jsonporter pushed a commit that referenced this pull request Feb 14, 2023
* chore: fix expander bug

Signed-off-by: Carina Ursu <[email protected]>

* chore: add await everywhere

Signed-off-by: Carina Ursu <[email protected]>

---------

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: Jason Porter <[email protected]>
jsonporter added a commit that referenced this pull request Feb 14, 2023
* fix: execution view children fetch on demand refactor

Signed-off-by: Olga Nad <[email protected]>
Signed-off-by: Jason Porter <[email protected]>

* fix: incorrect import

Signed-off-by: Olga Nad <[email protected]>
Signed-off-by: Jason Porter <[email protected]>

* chore: fix expander bug (#677)

* chore: fix expander bug

Signed-off-by: Carina Ursu <[email protected]>

* chore: add await everywhere

Signed-off-by: Carina Ursu <[email protected]>

---------

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: Jason Porter <[email protected]>

* fix: toggle in timeline

Signed-off-by: Olga Nad <[email protected]>
Signed-off-by: Jason Porter <[email protected]>

* fix: checkForDynamicExecutions

Signed-off-by: Jason Porter <[email protected]>

* Revert "fix: checkForDynamicExecutions"

This reverts commit 450d144.

Signed-off-by: Jason Porter <[email protected]>

* Perf Refactor: Some parent nodes don't load children (#680)

fix: checkForDynamicExecutions
Signed-off-by: Jason Porter <[email protected]>

* chore: simplify graph code

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: Jason Porter <[email protected]>

* adding util

Signed-off-by: Jason Porter <[email protected]>

* Changing type

Signed-off-by: Jason Porter <[email protected]>

* Updated click handlers

Signed-off-by: Jason Porter <[email protected]>

* Fixed graph dynamic node issue

Signed-off-by: Jason Porter <[email protected]>

* Key was not a valid prop in this case, removed

Signed-off-by: Jason Porter <[email protected]>

* chore: fix tests

Signed-off-by: Carina Ursu <[email protected]>
Signed-off-by: Jason Porter <[email protected]>

---------

Signed-off-by: Olga Nad <[email protected]>
Signed-off-by: Jason Porter <[email protected]>
Signed-off-by: Carina Ursu <[email protected]>
Co-authored-by: Carina Ursu <[email protected]>
Co-authored-by: James <[email protected]>
Co-authored-by: james-union <[email protected]>
Co-authored-by: Jason Porter <[email protected]>
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.

3 participants