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.

Sign up for GitHub

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

Jump to bottom

ConfirmPage: update setApprovalForAll dialog copy #15744

Merged
merged 2 commits into from
Sep 8, 2022
Merged

ConfirmPage: update setApprovalForAll dialog copy #15744

merged 2 commits into from
Sep 8, 2022

Conversation

digiwand
Copy link
Contributor

@digiwand digiwand commented Sep 6, 2022

Explanation

Updates ConfirmPage setApprovalForAll warning dialog copy. See #15512 (comment) for details

More Information

Follow-up: #15512

Screenshots/Screencaps

Before

183882919-ca3f15b4-dd5e-4751-a9bb-07d91567fa30

After

Screenshot 2022-09-06 at 6 44 02 PM

Manual Testing Steps

  • send a setApprovalForAll transaction and observe warning dialog copy

@digiwand digiwand requested a review from a team as a code owner September 6, 2022 10:51
@digiwand digiwand requested a review from PeterYinusa September 6, 2022 10:51
@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2022

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

digiwand
digiwand commented Sep 6, 2022
View reviewed changes
ui/components/app/confirm-page-container/confirm-page-container.component.js
{t('confirmPageDialogSetApprovalForAll')}
{/*
TODO: https://github.com/MetaMask/metamask-extension/issues/15745
style={{ fontWeight: 'bold' }} because reset.scss removes font-weight from b. We should fix this.
Copy link
Contributor Author

@digiwand digiwand Sep 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I don't have bandwidth to propose an update to restore the font-weight for our <b> elements right now. This is a TODO item

see: #15745

@digiwand digiwand mentioned this pull request Sep 6, 2022
Merged
2 tasks
@metamaskbot
Copy link
Collaborator

Builds ready [39393e4]
Page Load Metrics (1699 ± 47 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint91145113147
domContentLoaded1561188016829345
load1561190416999947
domInteractive1561188016829345

highlights:

storybook

@digiwand digiwand merged commit a7ff659 into develop Sep 8, 2022
@digiwand digiwand deleted the setApprovalForAll-update-copy branch September 8, 2022 10:17
@github-actions github-actions bot locked and limited conversation to collaborators Sep 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@coreyjanssen coreyjanssen coreyjanssen approved these changes

@jpuri jpuri jpuri approved these changes

@PeterYinusa PeterYinusa Awaiting requested review from PeterYinusa PeterYinusa is a code owner automatically assigned from MetaMask/extension-devs

@adonesky1 adonesky1 Awaiting requested review from adonesky1

Assignees
No one assigned
Labels
area-NFTs area-transactions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@digiwand @metamaskbot @jpuri @coreyjanssen