Skip to content
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

Detach navigator from sidebar #9933

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

NickWingate
Copy link
Contributor

  • Resolves: #
  • Target version: master

Summary

TODO

  • ...

Checklist

  • I have run make prettier-write and formatted the code.
  • All commits have Change-Id
  • I have run tests with make check
  • I have issued make run and manually verified that everything looks okay
  • Documentation (manuals or wiki) has been updated or is not required

Signed-off-by: NickWingate <[email protected]>
Change-Id: Iba2142d44f29ad0eb9293859f45c0718d3a223b4
Signed-off-by: NickWingate <[email protected]>
Change-Id: Ic1ee078dca4792b314e5f9a835ae50a79ab7ee62
Signed-off-by: NickWingate <[email protected]>
Change-Id: Ia09748abcec00a75b8743778907d5ede28c6720b
Responsible for handling and rendering events
related to the navigator

Signed-off-by: NickWingate <[email protected]>
Change-Id: I1bd3718f1cd3ab4e46e6354166267376013588a8
*/

/* global app */
class NavigatorPanel {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we now create base class: SidebarBase which will be parent of Sidebar and Navigator.
We can then share almost all of the onAdd, onRemove, isVisible, onJSUpdate, onJSAction.
We only need to define strings which are different:

this.commandId = 'navigator' / 'sidebar';
this.containerWrapperId = 'navigator-dock-wrapper' / 'siebar-dock-wrapper';
this.panelId = 'navigator-panel' / 'sidebar-panel';

why do not concatenate even more: TYPE + '-dock-wrapper' ? because it's then easier to git grep if id is fully present in the code and css to find related code :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To Review
Development

Successfully merging this pull request may close these issues.

3 participants