-
Notifications
You must be signed in to change notification settings - Fork 870
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
Assign Copy keyboard shortcut to 'copy clean link' (Mac App Menu part) #16746
Conversation
2c93b73
to
c88fcd3
Compare
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.
Approach/screenshot looks good.
c88fcd3
to
b5d1a4a
Compare
@@ -36,15 +40,30 @@ - (BOOL)shouldShowCleanLinkItem { | |||
return brave::HasSelectedURL([self getBrowser]); | |||
} | |||
|
|||
- (void)setKeyEquivalentToItem:(NSMenuItem*)item { |
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.
nit: I think this method can have bool args whether copy item or not.
3f79c43
to
0ede2dd
Compare
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.
I did not review the actual code changes, but screenshot and approach are 👍 from me.
0ede2dd
to
8c41151
Compare
Resolves brave/brave-browser#27902
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: