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

Update eth_accounts permission description #8693

Merged
merged 6 commits into from
May 28, 2020
Merged

Update eth_accounts permission description #8693

merged 6 commits into from
May 28, 2020

Conversation

rekmarks
Copy link
Member

@rekmarks rekmarks commented May 28, 2020 •
edited
Loading

Permissions Description

Original

View your public address (required)

  • Does not convey that multiple addresses can be viewed
  • What does "public" mean?

New

View the addresses of your permitted accounts (required)

  • Conveys that multiple addresses can be viewed
  • Conveys that we're talking about account addresses
  • "permitted" addresses (as opposed to "public") are the addresses that you're permitting!
    • Optionally, could change to "Connected" for now, but I don't like the term
      • I'd love for us to change the language of "Connected" to "Permitted" in some auspicious future

Additional Changes

  • Changes the names of variables used with t to get permissions description locale messages to permissionName, so that they're easier to search for.
  • Ensures that the connected accounts popover permissions list handles longer permission names.

@rekmarks rekmarks requested a review from a team as a code owner May 28, 2020 21:38
brad-decker
brad-decker previously approved these changes May 28, 2020
View reviewed changes
@brad-decker
Copy link
Contributor

I like it.

brad-decker
brad-decker previously approved these changes May 28, 2020
View reviewed changes
Copy link
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

I like it your.

Gudahtt
Gudahtt previously approved these changes May 28, 2020
View reviewed changes
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

This seems like a clear improvement!

We can always change it again if someone thinks of a better wording.

@rekmarks rekmarks dismissed stale reviews from Gudahtt and brad-decker via 379ca01 May 28, 2020 22:18
@rekmarks rekmarks requested review from Gudahtt and brad-decker May 28, 2020 22:19
@metamaskbot
Copy link
Collaborator

Builds ready [379ca01]
Page Load Metrics (621 ± 47 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint30443752
domContentLoaded3387416199747
load3407426219747
domInteractive3387416199747

@rekmarks rekmarks merged commit 606618e into develop May 28, 2020
@rekmarks rekmarks deleted the update-eth-accounts-description branch May 28, 2020 22:53
Gudahtt added a commit that referenced this pull request Jun 1, 2020
@Gudahtt
Merge remote-tracking branch 'origin/develop' into Version-v8.0.0
0187153 
* origin/develop: (689 commits)
  Implement asset page (#8696)
  fix crash on signature request (#8709)
  Fix accounts menu styling (#8707)
  Delete docs/porting_to_new_environment.md (#8704)
  Remove unused `getToErrorObject` parameters (#8705)
  hide connected-status on metamask ext (#8703)
  Stop adding permissions middleware to trusted connections (#8701)
  Use `send` state for send flow token (#8695)
  do not display extension id in connection modal (#8699)
  Fix tab content disappearing during scrolling on macOS Firefox (#8702)
  close details when button is pressed (#8694)
  Refactor token selectors (#8671)
  Update eth_accounts permission description (#8693)
  Extract selected token from token input (#8692)
  Fix propType for Home defaultHomeActiveTabName (#8683)
  Fix create account form styling (#8689)
  Remove unused `getSelectedTokenAssetImage` selector (#8691)
  Remove `getTxParams` (#8676)
  do not show account mismatch alert on details (#8678)
  Fix connect hardware styling (#8680)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gudahtt Gudahtt Gudahtt approved these changes

@brad-decker brad-decker Awaiting requested review from brad-decker

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rekmarks @brad-decker @metamaskbot @Gudahtt