Skip to content
This repository has been archived by the owner on Feb 2, 2024. It is now read-only.

[Batch Viewer] Add a Reset button + autoscale graph area #100

Merged
merged 8 commits into from
Jun 3, 2022

Conversation

alongoni
Copy link
Contributor

@alongoni alongoni commented May 23, 2022

Summary

Closes #48 and #44

image
image

To Test

  1. <> Open a tx on the BV page

@alongoni alongoni added app:Explorer Explorer App Enhancement New feature or request Medium Protofire labels May 23, 2022
@alongoni alongoni self-assigned this May 23, 2022
@alongoni alongoni changed the title 48 reset button batch viewer Add a Reset button on batch viewer May 23, 2022
@alongoni alongoni changed the title Add a Reset button on batch viewer [Batch Viewer] Add a Reset button May 23, 2022
@github-actions
Copy link

@coveralls
Copy link

coveralls commented May 23, 2022

Pull Request Test Coverage Report for Build 2437139669

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 21 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.1%) to 44.121%

Files with Coverage Reduction New Missed Lines %
src/utils/mediaQueries.ts 2 37.5%
src/components/Tooltip.tsx 19 28.0%
Totals Coverage Status
Change from base Build 2358811447: -0.1%
Covered Lines: 2172
Relevant Lines: 4190

💛 - Coveralls

@alongoni alongoni marked this pull request as ready for review May 24, 2022 18:35
@alongoni alongoni requested a review from matextrem May 24, 2022 18:35
@alongoni alongoni changed the title [Batch Viewer] Add a Reset button [Batch Viewer] Add a Reset button + autoscale graph area May 26, 2022

setElements(getNodes(txSettlement, networkId, heightSize))
}, [heightSize, error, isLoading, networkId, txSettlement])
setResetZoom(null)
}, [error, isLoading, txSettlement, networkId, heightSize, resetZoom])
Copy link
Contributor

@henrypalacios henrypalacios May 26, 2022

Choose a reason for hiding this comment

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

@alongoni I have used the resetZoom here as we had agreed. I also added the resize listener with the solution we discovered for issue #44 .

@alongoni alongoni requested a review from ramirotw May 26, 2022 14:22
@henrypalacios
Copy link
Contributor

@alongoni proposes to hide the button in the first render and activate it after modifying something in the graphic.

Since this requires an additional effort we have concluded to create a ticket to handle this in #101

ref.layout(getLayout()).run()
ref.fit()
}
ref.on('resize', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

@henrypalacios is this subscription properly cleaned when the chart is unmounted?

Copy link
Contributor

@henrypalacios henrypalacios Jun 2, 2022

Choose a reason for hiding this comment

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

The listeners seem to be removed when destroy the component, (I couldn't catch it from the documentation).
But with a thorough review of the Cy.Core object I got:

Selection_558

  1. I found that there were memory leaks for the events at interaction time. (I have corrected it)

  2. An untyped method that remove all the listener that we can use it to make sure that it is going to clean everything when unmounted.

src/env.d.ts Outdated Show resolved Hide resolved
@henrypalacios henrypalacios merged commit 3bb2446 into develop Jun 3, 2022
@henrypalacios henrypalacios deleted the 48-reset-button-batch-viewer branch June 3, 2022 21:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
app:Explorer Explorer App Enhancement New feature or request Medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Batch viewer: add 'reset to default state' button to the diagram
6 participants