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

[FLASK] Add tooltips to show info about a permission #17685

Merged
merged 12 commits into from
Mar 20, 2023

Conversation

Mrtenz
Copy link
Member

@Mrtenz Mrtenz commented Feb 9, 2023

Explanation

This adds support for showing tooltips on permissions, and adds a tooltip to all the Snaps-related permissions.

Closes #17624.

Before

image

After

image image

Manual Testing Steps

  1. Go to test-snaps.
  2. Click to connect to a snap.
  3. Ensure that the right icon shows up, and shows a tooltip when hovering.

Pre-merge author checklist

  • I've clearly explained:
    • What problem this PR is solving
    • How this problem was solved
    • How reviewers can test my changes
  • Sufficient automated test coverage has been added

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

ui/helpers/utils/permission.js Outdated Show resolved Hide resolved
GuillaumeRx
GuillaumeRx previously approved these changes Feb 9, 2023
@GuillaumeRx
Copy link
Contributor

Code LGTM, build is not passing though

@metamaskbot
Copy link
Collaborator

Builds ready [22551a5]
Page Load Metrics (1502 ± 136 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint981821342210
domContentLoaded102719731474295141
load108419741502283136
domInteractive102719731474295141
Bundle size diffs
  • background: 0 bytes
  • ui: 661 bytes
  • common: 0 bytes

@metamaskbot
Copy link
Collaborator

Builds ready [22551a5]
Page Load Metrics (1502 ± 136 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint981821342210
domContentLoaded102719731474295141
load108419741502283136
domInteractive102719731474295141
Bundle size diffs
  • background: 0 bytes
  • ui: 661 bytes
  • common: 0 bytes

@Mrtenz Mrtenz force-pushed the mrtenz/permission-tooltips branch 2 times, most recently from cba6ec9 to ad73413 Compare February 16, 2023 10:34
@Mrtenz Mrtenz force-pushed the mrtenz/permission-tooltips branch from 933c756 to 7919b6a Compare March 16, 2023 15:10
@metamaskbot
Copy link
Collaborator

Builds ready [b61482d]
Page Load Metrics (1698 ± 79 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1024011386330
domContentLoaded14282029167717383
load14992029169816579
domInteractive14282029167717383
Bundle size diffs
  • background: 0 bytes
  • ui: 529 bytes
  • common: 0 bytes

@codecov
Copy link

codecov bot commented Mar 16, 2023

Codecov Report

Merging #17685 (803bd62) into develop (c940f74) will decrease coverage by 0.03%.
The diff coverage is 8.00%.

@@             Coverage Diff             @@
##           develop   #17685      +/-   ##
===========================================
- Coverage    63.96%   63.93%   -0.03%     
===========================================
  Files          914      914              
  Lines        35621    35639      +18     
  Branches      9028     9035       +7     
===========================================
+ Hits         22783    22785       +2     
- Misses       12838    12854      +16     
Impacted Files Coverage Δ
...ission-list/permissions-connect-permission-list.js 12.50% <0.00%> (ø)
ui/helpers/utils/permission.js 5.81% <8.70%> (+1.40%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Mrtenz Mrtenz marked this pull request as ready for review March 16, 2023 16:24
@Mrtenz Mrtenz requested a review from a team as a code owner March 16, 2023 16:24
@Mrtenz Mrtenz requested a review from digiwand March 16, 2023 16:24
app/_locales/en/messages.json Show resolved Hide resolved
app/_locales/en/messages.json Show resolved Hide resolved
ui/helpers/utils/permission.js Outdated Show resolved Hide resolved
ui/helpers/utils/permission.js Outdated Show resolved Hide resolved
@Mrtenz Mrtenz force-pushed the mrtenz/permission-tooltips branch from 9e06362 to 03928f2 Compare March 17, 2023 12:59
@Mrtenz Mrtenz force-pushed the mrtenz/permission-tooltips branch from 03928f2 to 58a5ff3 Compare March 20, 2023 10:28
@metamaskbot
Copy link
Collaborator

Builds ready [803bd62]
Page Load Metrics (1719 ± 67 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1031701262110
domContentLoaded15062045171314168
load15252045171914067
domInteractive15062045171314168
Bundle size diffs
  • background: 0 bytes
  • ui: 617 bytes
  • common: 0 bytes

@Mrtenz Mrtenz merged commit c59e2b4 into develop Mar 20, 2023
@Mrtenz Mrtenz deleted the mrtenz/permission-tooltips branch March 20, 2023 11:32
@github-actions github-actions bot locked and limited conversation to collaborators Mar 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FLASK] Add permission info popups
4 participants