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

Fix 12458 v2 #13594

Closed
wants to merge 4 commits into from
Closed

Fix 12458 v2 #13594

wants to merge 4 commits into from

Conversation

jayyyin
Copy link

@jayyyin jayyyin commented Mar 25, 2018

my second attempt at fixing #12458
the code is a bit cleaner this time and I've managed to get the desired selection in the menu but I can't figure out how to make the dropdown menu appear, been trying to add my own windowAction to see if that would work, but came to the realization there just isn't the feature to expand the menu from outside the menubar.js object, any help here would be appreciated.

@codecov-io
Copy link

codecov-io commented Mar 25, 2018

Codecov Report

Merging #13594 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master   #13594      +/-   ##
==========================================
- Coverage   56.74%   56.71%   -0.03%     
==========================================
  Files         285      285              
  Lines       28797    28810      +13     
  Branches     4755     4757       +2     
==========================================
- Hits        16341    16340       -1     
- Misses      12456    12470      +14
Flag Coverage Δ
#unittest 56.71% <0%> (-0.03%) ⬇️
Impacted Files Coverage Δ
app/common/constants/keyCodes.js 100% <ø> (ø) ⬆️
js/constants/windowConstants.js 100% <ø> (ø) ⬆️
js/actions/windowActions.js 4.9% <0%> (-0.05%) ⬇️
app/renderer/components/navigation/menuBar.js 19.09% <0%> (-1.5%) ⬇️
js/stores/windowStore.js 28.48% <0%> (-0.28%) ⬇️

@bsclifton
Copy link
Member

Closing PR as stale- apologies that we haven't had a chance to review and help you solve the remaining work ☹️ Unfortunately, the Alt + menu navigation code is extremely custom and is not easy to change. This will be fixed as we move to Brave Core

@bsclifton bsclifton closed this Jul 22, 2018
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.

None yet

3 participants