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

add access button to the BV #73

Merged
merged 2 commits into from
May 2, 2022
Merged

add access button to the BV #73

merged 2 commits into from
May 2, 2022

Conversation

alongoni
Copy link
Contributor

@alongoni alongoni commented Apr 26, 2022 •
edited
Loading

Summary

Closes #64
image
image

To Test

  1. <> Open the page Order detail
    Check the appearance of the button "View batch graph". Later we will be adding a link to the specific batch.

@alongoni alongoni added app:Explorer Explorer App Enhancement New feature or request Protofire labels Apr 26, 2022
@alongoni alongoni self-assigned this Apr 26, 2022
@coveralls
Copy link

coveralls commented Apr 26, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 2227224200

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.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 44.106%
Totals Coverage Status
Change from base Build 2220797514: 0.0%
Covered Lines: 2155
Relevant Lines: 4150
💛 - Coveralls

@github-actions
Copy link

@alongoni alongoni marked this pull request as ready for review April 26, 2022 18:54
elena-zh
elena-zh approved these changes Apr 27, 2022
View reviewed changes
Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

LGTM!

@elena-zh
Copy link

@alongoni , the only thing that I noticed here is that changes for the 'Copy' feature from #68 PR are not applied here

@alongoni alongoni requested a review from ramirotw April 27, 2022 13:06
ramirotw
ramirotw suggested changes Apr 27, 2022
View reviewed changes
Copy link
Contributor

@ramirotw ramirotw left a comment

Choose a reason for hiding this comment

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

Almost there, and please point the base branch to 41-anchor-url which is @henrypalacios's branch

@alongoni alongoni changed the base branch from develop to 41-anchor-url April 27, 2022 22:06
@alongoni alongoni requested a review from ramirotw April 29, 2022 14:11
ramirotw
ramirotw suggested changes Apr 29, 2022
View reviewed changes
Copy link
Contributor

@ramirotw ramirotw left a comment

Choose a reason for hiding this comment

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

could we move all the changes in the links and package.json to a different PR?

@alongoni alongoni force-pushed the 64-access-to-batch-viewer branch from ef2af92 to 36d60f1 Compare April 29, 2022 21:18
@alongoni alongoni requested a review from ramirotw April 29, 2022 21:20
ramirotw
ramirotw suggested changes Apr 30, 2022
View reviewed changes
Copy link
Contributor

@ramirotw ramirotw left a comment

Choose a reason for hiding this comment

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

nice!! just 1 little thing and it's good to go.

Remove the underline during on-hover

image

@alongoni alongoni requested a review from ramirotw May 2, 2022 12:03
@alongoni alongoni merged commit d9e568d into 41-anchor-url May 2, 2022
@alongoni alongoni deleted the 64-access-to-batch-viewer branch May 2, 2022 13:08
@elena-zh
Copy link

elena-zh commented May 3, 2022

Hey @alongoni , I notices this weird issue in this PR
image

It is reproducible when open TX list first, then to switch to the BV. See the video: https://watch.screencastify.com/v/15HqO9IiqlC1DV1dOTbt

But I was not able to reproduce the issue in #49 PR , so seems like it is fixed there.
But could you please take a look and confirm that there is no need to worry about it?

Thanks!

@ramirotw
Copy link
Contributor

ramirotw commented May 3, 2022

@elena-zh that's correct, the base branch for this PR was set to @henrypalacios' branch, but when this PR was merged the issue you are pointing out was still present (now it's fixed)

henrypalacios added a commit that referenced this pull request May 4, 2022
@henrypalacios @alongoni
Adding useQuery search URL params hook (#49)
8ca4258 
* Adding useQuery URL's params hook

* removing button and switching to tabs

* Adding TabIcon component

* add access button to the BV (#73)

* add Link to BV

* remove underline

* remove border on BV and fix mobile view

* Fix rendering problem when changing tab

* Fix memory leak with cytoscape-popper

* Singular name suggestion, simplifying the enum

* take advantage of string enum comparison

Co-authored-by: Agustín Longoni <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ramirotw ramirotw ramirotw approved these changes

@elena-zh elena-zh elena-zh approved these changes

@henrypalacios henrypalacios Awaiting requested review from henrypalacios

Assignees

@alongoni alongoni

Labels
app:Explorer Explorer App Enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add an access to the Batch Viewer on order page
4 participants
@alongoni @coveralls @elena-zh @ramirotw