Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Put back in handler for null/empty context menu detail. This is requi… #6250

Merged
merged 1 commit into from
Dec 16, 2016
Merged

Put back in handler for null/empty context menu detail. This is requi… #6250

merged 1 commit into from
Dec 16, 2016

Conversation

bsclifton
Copy link
Member

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Put back in handler for null/empty context menu detail. This is required in order to hide any ContextMenu object

Caused by 7fb2e4c#diff-733d30fbf34816322d3f25dd59c1eb99

Fixes #6236

Auditors: @bridiver, @darkdh

Test Plan:

  1. Launch Brave and use the kabob / cheeseburger icon
  2. Notice the context menu comes up
  3. Click anywhere on the screen (outside of the context menu)
  4. Notice context menu closes itself

@bsclifton bsclifton added this to the 0.13.0 milestone Dec 16, 2016
@@ -664,6 +664,8 @@ const doAction = (action) => {
if (action.notify) {
ipc.send('autofill-popup-hidden', action.tabId)
}
} else {
windowState = windowState.delete('contextMenuDetail')
Copy link
Member

Choose a reason for hiding this comment

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

I think we should just move L663 here. Just like reverting the part of this commit 7fb2e4c#diff-733d30fbf34816322d3f25dd59c1eb99L651

Copy link
Member Author

Choose a reason for hiding this comment

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

@darkdh OK fixed- because I am not sure if the order matters, I moved the delete to above this check

…red in order to hide any ContextMenu object

Caused by 7fb2e4c#diff-733d30fbf34816322d3f25dd59c1eb99

Fixes #6236

Auditors: @bridiver, @darkdh

Test Plan:
1. Launch Brave and use the kabob / cheeseburger icon
2. Notice the context menu comes up
3. Click anywhere on the screen (outside of the context menu)
4. Notice context menu closes itself
@bsclifton
Copy link
Member Author

Merging... I can implement any other feedback in a follow up; let me know 😄

@bsclifton bsclifton merged commit 71bc241 into brave:master Dec 16, 2016
@bsclifton bsclifton deleted the hamburger-hold-the-pickles branch December 16, 2016 17:12
@luixxiul
Copy link
Contributor

Did this fix #6253 too?

@bsclifton
Copy link
Member Author

@luixxiul it will fix issues with regards to the menu- but I am not sure it fixes the other buttons. I'll update here once I can try

@bsclifton
Copy link
Member Author

@luixxiul buttons work OK for me? I'm not sure if it's a result of this or not though

@luixxiul
Copy link
Contributor

CC @srirambv

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Menu will not disappear
3 participants