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

Redux component - Menubar #8652

Closed
NejcZdovc opened this issue May 3, 2017 · 7 comments
Closed

Redux component - Menubar #8652

NejcZdovc opened this issue May 3, 2017 · 7 comments
Assignees
@NejcZdovc
Labels
needs-info Another team member needs information from the PR/issue opener. OS/Windows QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified refactoring/redux-component release-notes/include
Milestone
0.15.300 (Frozen,...

Comments

@NejcZdovc
Copy link
Contributor

NejcZdovc commented May 3, 2017 •
edited by luixxiul
Loading

Test plan

#8656 (comment)

Describe the issue you encountered:
Refactor Menubar into redux component.
@NejcZdovc NejcZdovc added this to the 0.15.2 milestone May 3, 2017
@NejcZdovc NejcZdovc self-assigned this May 3, 2017
@bsclifton
Copy link
Member

bsclifton commented May 3, 2017 •
edited
Loading

I would vote to push this past 1.0 because it's very limited (only used on Windows; even then, only when menu is visible). I don't think we'd get much by reworking it

What do you think?

@NejcZdovc
Copy link
Contributor Author

Sadly I need to refactor this, so that I can refactor #8651 and then when #8651 is refactored, #8640 will be fixed 😃

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue May 3, 2017
@NejcZdovc
Refactors MenuBar and MenuBarItem into redux component
f5d7071 
Resolves brave#8652

Auditors: @bsclifton @bridiver

Test Plan:
- open menu on windows
- try clicking around
- everything should behave as did before
@NejcZdovc NejcZdovc mentioned this issue May 3, 2017
Merged
4 tasks
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue May 3, 2017
@NejcZdovc
Refactors MenuBar and MenuBarItem into redux component
db5ee4b 
Resolves brave#8652

Auditors: @bsclifton @bridiver

Test Plan:
- open menu on windows
- try clicking around
- everything should behave as did before
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue May 3, 2017
@NejcZdovc
Refactors MenuBar and MenuBarItem into redux component
4763789 
Resolves brave#8652

Auditors: @bsclifton @bridiver

Test Plan:
- open menu on windows
- try clicking around
- everything should behave as did before
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue May 3, 2017
@NejcZdovc
Refactors MenuBar and MenuBarItem into redux component
b8c397f 
Resolves brave#8652

Auditors: @bsclifton @bridiver

Test Plan:
- open menu on windows
- try clicking around
- everything should behave as did before
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue May 3, 2017
@NejcZdovc
Refactors MenuBar and MenuBarItem into redux component
e04031f 
Resolves brave#8652

Auditors: @bsclifton @bridiver

Test Plan:
- open menu on windows
- try clicking around
- everything should behave as did before
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue May 3, 2017
@NejcZdovc
Refactors MenuBar and MenuBarItem into redux component
7dcb919 
Resolves brave#8652

Auditors: @bsclifton @bridiver

Test Plan:
- open menu on windows
- try clicking around
- everything should behave as did before
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue May 4, 2017
@NejcZdovc
Refactors MenuBar and MenuBarItem into redux component
5fc6dff 
Resolves brave#8652

Auditors: @bsclifton @bridiver

Test Plan:
- open menu on windows
- try clicking around
- everything should behave as did before
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue May 4, 2017
@NejcZdovc
Refactors MenuBar and MenuBarItem into redux component
ab6785a 
Resolves brave#8652

Auditors: @bsclifton @bridiver

Test Plan:
- open menu on windows
- try clicking around
- everything should behave as did before
@bridiver
Copy link
Collaborator

bridiver commented May 5, 2017

sorry, but we need to revert this for now. It stores methods in the state and we can't do that because they are not serializable

@bridiver
Copy link
Collaborator

bridiver commented May 5, 2017

I guess the menu template thing has been there for a long time so I'll leave this for now, but we need to fix that so I'll open a ticket

@NejcZdovc
Copy link
Contributor Author

@bridiver yeah I listed this problem here #8528, because it would be overkill to refactor it in this PR. Problem is that the whole context menu is problematic and we need to find a better way to handle it.

This was referenced May 5, 2017
Closed
Closed
Closed
Closed
@luixxiul
Copy link
Contributor

luixxiul commented May 6, 2017

@bridiver @NejcZdovc so does this issue have to be reopened? I'm asking it for QA work.

@luixxiul luixxiul added the needs-info Another team member needs information from the PR/issue opener. label May 6, 2017
@NejcZdovc
Copy link
Contributor Author

@luixxiul no need to be reopened, if you are talking about context menu, we will address this issue later on.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@NejcZdovc NejcZdovc

Labels
needs-info Another team member needs information from the PR/issue opener. OS/Windows QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified refactoring/redux-component release-notes/include
Projects
None yet
Milestone
0.15.300 (Frozen, only critical adds ...
Development

No branches or pull requests
6 participants
@bridiver @alexwykoff @luixxiul @bsclifton @NejcZdovc @srirambv