Approve Screen Design QA #1390

rickycodes · 2020-02-28T20:12:47Z

Description

re: #1330

Approve

Approve 2

Edit Transaction Fee

Edit Transaction Fee 2

Edit Permission

I've also removed the ApproveSuccess screen

Checklist

There is a related GitHub issue
Tests are included if applicable
Any added code is fully documented (not applicable, nothing was added)
Use 24px padding (or as low as 22px for smaller screen) to the left and right of the screen.
This includes address bar and large cancel/approve buttons.
The modal window looks like it goes over 24px.
Remove confirm page.
Include screen shots.

Issue

Resolves #1330

cjeria

LGTM

estebanmino · 2020-03-03T15:35:26Z

app/components/UI/ActionModal/index.js

 	},
 	modalContainer: {
-		width: '90%',
+		width: '100%',


this component is used in other places of the app not only here, is this working everywhere?

yep, it's working as expected everywhere. I actually tested it via the Send Feedback link as well (since that's easier to get to).

estebanmino · 2020-03-03T15:35:52Z

app/components/UI/ActionView/index.js

@@ -12,7 +12,8 @@ const styles = StyleSheet.create({
 		borderTopWidth: 1,
 		flex: 0,
 		flexDirection: 'row',
-		padding: 16
+		paddingVertical: 16,
+		paddingHorizontal: 24


same here, component re used in other places

we still want the same padding regardless of where it's used

estebanmino · 2020-03-03T15:38:31Z

app/components/UI/TransactionEdit/index.js

-		right: 15,
-		left: 15,
+		right: 24,
+		left: 24,


this component is not related to approve screen, there are other updates as well?

you're right that this one is not in the screens included in the PR, but the spacing on these still needed to be adjusted to match

estebanmino · 2020-03-03T15:39:25Z

app/components/Views/PaymentChannel/PaymentChannelDeposit/index.js

@@ -138,7 +138,7 @@ const styles = StyleSheet.create({
 		borderColor: colors.grey100,
 		borderRightWidth: 1,
 		paddingRight: 35,
-		paddingLeft: 20
+		paddingLeft: 24


same here, not approve

the from graphic is in the screens and needed to be moved over slightly to fall into the same grid alignment.

estebanmino

looks good, my only doubt is I see changes in other places that aren't related to approve screen

rickycodes · 2020-03-03T15:57:46Z

I've addressed your feedback. there are a couple spacing changes that are unrelated to the approve screen, but this spacing should be consistent throughout the application.

ibrahimtaveras00 · 2020-03-04T20:42:58Z

I checked on various screen sizes on both OS's and the only issues I could find are the following:

Happening on a smaller screen (iPhone SE) but looks fine on the small screen on Nexus S

The blue check mark on smaller screens:

Here's iPhone SE:

Here's Nexus S:

And the "E" for "AVERAGE" on this view:

which is happening on both iPhone SE and Nexus S

steps to get to this view:

go to browser
switch to ropsten network
go to https://faucet.metamask.io/
after connecting attempt to donate some ETH
on confirm view, tap on Edit on the top left

cjeria · 2020-03-04T21:03:28Z

good catch @ibrahimtaveras00 ! I'll have a look at this asap!

cjeria · 2020-03-05T02:31:36Z

Here are suggested solutions to all of the above UI bugs and link to figma file below. Let me know if there are any questions! cc @rickycodes . We could ignore the new button styles for now since it's out of scope, but we'll eventually want to update them.

Figma file
https://www.figma.com/file/1aRs60GBYLLlBkReWg3XSA/Gas-selection-button-group?node-id=1%3A281

estebanmino

From yesterday's conversations, for this PR we have to address #1390 (comment) and apply @cjeria design in a separate PR

rickycodes · 2020-03-11T15:59:13Z

Closing this and will open a new PR with this work as well as the new work suggested in #1390 (comment)

Changes to the gas selection button group as requested here: #1390 (comment)

Changes to the gas selection button group as requested here: MetaMask/metamask-mobile#1390 (comment)

rickycodes changed the title ~~Adjust padding~~ Approve Screen Design QA Feb 28, 2020

rickycodes force-pushed the approve-screen-design-qa branch from 7431298 to 1b0e5d5 Compare March 1, 2020 00:42

rickycodes marked this pull request as ready for review March 1, 2020 20:07

rickycodes added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Mar 1, 2020

rickycodes requested review from cjeria and estebanmino March 1, 2020 20:08

cjeria approved these changes Mar 2, 2020

View reviewed changes

rickycodes requested a review from ibrahimtaveras00 March 3, 2020 01:08

estebanmino reviewed Mar 3, 2020

View reviewed changes

rickycodes added 6 commits March 3, 2020 14:42

Adjust padding

73e52a9

Add padding for ActionModal

0c9833c

Do not navigate to ApproveSuccessView

da4c70b

Update tests

6cce8a6

Remove ApproveSuccessView

e620311

Update tests

c65141a

rickycodes force-pushed the approve-screen-design-qa branch from 338f069 to c65141a Compare March 3, 2020 19:42

rickycodes mentioned this pull request Mar 3, 2020

Add vertical padding to close button #1398

Merged

3 tasks

estebanmino suggested changes Mar 6, 2020

View reviewed changes

rickycodes closed this Mar 11, 2020

rickycodes mentioned this pull request Mar 24, 2020

Approve design updates #1455

Merged

rickycodes added a commit that referenced this pull request Mar 27, 2020

Approve design updates (#1455)

b2cabde

Changes to the gas selection button group as requested here: #1390 (comment)

cronny25 pushed a commit to cronny25/metamask-mobile that referenced this pull request Jul 27, 2022

Approve design updates (#1455)

2013040

Changes to the gas selection button group as requested here: MetaMask/metamask-mobile#1390 (comment)

rickycodes deleted the approve-screen-design-qa branch November 21, 2022 00:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Approve Screen Design QA #1390

Approve Screen Design QA #1390

rickycodes commented Feb 28, 2020 •

edited

Loading

cjeria left a comment

estebanmino Mar 3, 2020

rickycodes Mar 3, 2020

estebanmino Mar 3, 2020

rickycodes Mar 3, 2020

estebanmino Mar 3, 2020

rickycodes Mar 3, 2020 •

edited

Loading

estebanmino Mar 3, 2020

rickycodes Mar 3, 2020 •

edited

Loading

estebanmino left a comment

rickycodes commented Mar 3, 2020 •

edited

Loading

ibrahimtaveras00 commented Mar 4, 2020 •

edited by rickycodes

Loading

cjeria commented Mar 4, 2020

cjeria commented Mar 5, 2020 •

edited

Loading

estebanmino left a comment

rickycodes commented Mar 11, 2020

Approve Screen Design QA #1390

Approve Screen Design QA #1390

Conversation

rickycodes commented Feb 28, 2020 • edited Loading

cjeria left a comment

Choose a reason for hiding this comment

estebanmino Mar 3, 2020

Choose a reason for hiding this comment

rickycodes Mar 3, 2020

Choose a reason for hiding this comment

estebanmino Mar 3, 2020

Choose a reason for hiding this comment

rickycodes Mar 3, 2020

Choose a reason for hiding this comment

estebanmino Mar 3, 2020

Choose a reason for hiding this comment

rickycodes Mar 3, 2020 • edited Loading

Choose a reason for hiding this comment

estebanmino Mar 3, 2020

Choose a reason for hiding this comment

rickycodes Mar 3, 2020 • edited Loading

Choose a reason for hiding this comment

estebanmino left a comment

Choose a reason for hiding this comment

rickycodes commented Mar 3, 2020 • edited Loading

ibrahimtaveras00 commented Mar 4, 2020 • edited by rickycodes Loading

cjeria commented Mar 4, 2020

cjeria commented Mar 5, 2020 • edited Loading

estebanmino left a comment

Choose a reason for hiding this comment

rickycodes commented Mar 11, 2020

rickycodes commented Feb 28, 2020 •

edited

Loading

rickycodes Mar 3, 2020 •

edited

Loading

rickycodes Mar 3, 2020 •

edited

Loading

rickycodes commented Mar 3, 2020 •

edited

Loading

ibrahimtaveras00 commented Mar 4, 2020 •

edited by rickycodes

Loading

cjeria commented Mar 5, 2020 •

edited

Loading