-
Notifications
You must be signed in to change notification settings - Fork 869
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
Add permission duration to connect from panel on desktop #20008
Conversation
8bc7f5d
to
dfbc4e9
Compare
A Storybook has been deployed to preview UI for the latest push |
A Storybook has been deployed to preview UI for the latest push |
025f593
to
857f4fe
Compare
A Storybook has been deployed to preview UI for the latest push |
857f4fe
to
3ca0a42
Compare
A Storybook has been deployed to preview UI for the latest push |
A Storybook has been deployed to preview UI for the latest push |
ee601a0
to
8c0899c
Compare
A Storybook has been deployed to preview UI for the latest push |
8c0899c
to
d239b7a
Compare
A Storybook has been deployed to preview UI for the latest push |
A Storybook has been deployed to preview UI for the latest push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C++ core changes ++
|
||
base::android::ScopedJavaLocalRef<jobject> GetJavaBoolean( | ||
JNIEnv* env, | ||
const bool& native_bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just bool native_bool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ Android side
A Storybook has been deployed to preview UI for the latest push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iOS++
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frontend approved with suggestions
handler.on(PanelActions.requestSitePermission.type, | ||
async(store: Store, payload: RequestSitePermissionPayloadType) => { | ||
const apiProxy = getWalletPanelApiProxy() | ||
await apiProxy.panelHandler.requestPermission(payload.accountId) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this go in a mutation instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this works, I suggest merging it as-is and we can convert the handler to a mutation in another PR.
bd33833
to
0f53daf
Compare
0f53daf
to
09165d5
Compare
A Storybook has been deployed to preview UI for the latest push |
Resolves brave/brave-browser#32643
When users click on connect, we will create a permission request same as dapp request permission and rest of the flow is the same.
This also deprecates AddPermission API to prevent permission is being added without permission duration tracking.
@AlexeyBarabash addressed
AddPermission
deprecation on Android@Douglashdaniel added active account indicator to v2 panel on desktop
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run lint
,npm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
Panel Permission duration
Active account indicator
Panel
and then openDApp Connection Settings
Account
selector.Active
indicator for the current selected accountActive
indicator should update correctly.Screen.Recording.10.mov