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

Fix activity log inline buttons #8908

Merged
merged 1 commit into from
Jul 3, 2020
Merged

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jul 3, 2020

The inline speedup and speedup cancellation buttons in the activity log were broken. An exception would be thrown upon either button being clicked, and nothing would happen from the user's perspective.

Both handlers were being passed a transaction id, which was a holdover from before the transaction list redesign. The handlers passed for these two actions now have the transaction id embedded, so it doesn't need to be passed in anymore. They expect the click event to be passed through instead.

The handlers passed also didn't handle closing the transaction details modal when clicked. After fixing the first problem, they still didn't work because the speedup/cancel dialog was shown behind the transaction details modal.

Both issues are now fixed. Both buttons now close the transaction details modal, and trigger the appropriate action.

The inline speedup and speedup cancellation buttons in the activity log
were broken. An exception would be thrown upon either button being
clicked, and nothing would happen from the user's perspective.

Both handlers were being passed a transaction id, which was a holdover
from before the transaction list redesign. The handlers passed for
these two actions now have the transaction id embedded, so it doesn't
need to be passed in anymore. They expect the click event to be passed
through instead.

The handlers passed also didn't handle closing the transaction details
modal when clicked. After fixing the first problem, they still didn't
work because the speedup/cancel dialog was shown behind the transaction
details modal.

Both issues are now fixed. Both buttons now close the transaction
details modal, and trigger the appropriate action.
@Gudahtt Gudahtt force-pushed the fix-activity-log-inline-buttons branch from 8136c83 to 059a5b2 Compare July 3, 2020 15:16
@Gudahtt Gudahtt marked this pull request as ready for review July 3, 2020 15:16
@Gudahtt Gudahtt requested a review from a team as a code owner July 3, 2020 15:16
Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

LGTM! Good catch!

@metamaskbot
Copy link
Collaborator

Builds ready [059a5b2]
Page Load Metrics (680 ± 62 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint33110592713
domContentLoaded38887267912962
load38987368012962
domInteractive38787167812962

@Gudahtt Gudahtt merged commit 4a989c3 into develop Jul 3, 2020
@Gudahtt Gudahtt deleted the fix-activity-log-inline-buttons branch July 3, 2020 16:03
@metamaskbot metamaskbot mentioned this pull request Jul 3, 2020
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