Introduce View Container and base view components #338
Conversation
| parent.setLayout(gridLayout); | ||
|
|
||
| setupStaticMenuActions(); | ||
| updateChildView(activeView, ViewId.RE_AUTHENTICATE_VIEW); |
There was a problem hiding this comment.
Do we need both the active view and the view ID to be passed in?
| } | ||
|
|
||
| if (views.containsKey(newViewId)) { | ||
| BaseAmazonQView newView = views.get(newViewId); |
There was a problem hiding this comment.
Should we track the active view ID here for when the parent composite is disposed?
| private Disposable viewChangeEventSubscription; | ||
|
|
||
| public AmazonQViewContainer() { | ||
| viewChangeEventSubscription = Activator.getEventBroker().subscribe(ViewId.class, this); |
There was a problem hiding this comment.
You would need to subscribe to these events in the createPartControl every time if you dispose the subscription in the dispose call. I think Jonathan prefers not disposing the subscription.
|
|
||
| package software.aws.toolkits.eclipse.amazonq.views.router; | ||
|
|
||
| public enum ViewId { |
There was a problem hiding this comment.
I think this class was renamed in the ViewRouter PR so it might have gotten added accidentally.
|
|
||
| //default view passed in from router | ||
| //possible we'll use chatView as default? | ||
| activeView = views.get(currentActiveViewId); |
There was a problem hiding this comment.
This view might get disposed before being created if createPartControl calls updateChildView the first time. Not sure if that causes any issues.
| new AmazonQStaticActions(getViewSite()); | ||
| } | ||
|
|
||
| private void updateChildView() { |
There was a problem hiding this comment.
There might be race condition if both the createPartControl and onEvent call these methods simultaneously.
| * 2. viewContainer.init(currentView) | ||
| */ | ||
|
|
||
| public void initializeViews(final AmazonQViewType currentActiveViewId) { |
There was a problem hiding this comment.
Could we move this to the constructor?
Issue #336
Description of changes:
This PR introduces the view container component and a updated base view as part of a larger refactor of the view handling in the plugin. Changes include refactoring browser-less views to implement the new base view, and creating logic to display all content using a singular view. The code here is dependent on previous PR #309 and PR #337.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.