Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Decouple trace explorer #295

Merged
merged 4 commits into from
Mar 29, 2021
Merged

Decouple trace explorer #295

merged 4 commits into from
Mar 29, 2021

Conversation

bhufmann
Copy link
Collaborator

@bhufmann bhufmann commented Mar 9, 2021

This pull requests consists of multiple commits to decouple the trace explorer widgets into pure React components. These new React components will be part of the packages/react-components and can be used in other web applications independently to Theia.

  • ReactAnalysisWidget in packages/react-components/trace-explorer-analysis-widget.tsx
  • ReactOpenTracesWidget in packages/react-components/trace-explorer-opened-traces-widget.tsx

Along with this chain of commits, there are some code clean-ups and streamline the SignalManager and its usage.

@bhufmann bhufmann requested a review from MatthewKhouzam March 10, 2021 00:59
Copy link
Contributor

@MatthewKhouzam MatthewKhouzam left a comment

Choose a reason for hiding this comment

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

Works well

Copy link
Contributor

@MatthewKhouzam MatthewKhouzam left a comment

Choose a reason for hiding this comment

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

Works, thanks. Tested on Electron, browser (FF) and gitpod.

Copy link
Contributor

@MatthewKhouzam MatthewKhouzam left a comment

Choose a reason for hiding this comment

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

Works in Electron, Firefox and gitpod. (Github swallowed my last "approved")

Copy link
Contributor

@MatthewKhouzam MatthewKhouzam left a comment

Choose a reason for hiding this comment

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

Works here. Is the -impl suffix standard?

Copy link
Contributor

@MatthewKhouzam MatthewKhouzam left a comment

Choose a reason for hiding this comment

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

I am OK with this, it works on my machine, I want to make sure we are not moving farther away from undo-redo though.

@bhufmann
Copy link
Collaborator Author

Works here. Is the -impl suffix standard?

I just want to distinguish the file of the interface from the file of the implementation, since the file name is not equal to the class name like in Java. Besides this suffix is used in other places in the code base, trace-server-url-provider-frontend-impl.ts. That's where I got the idea.

@bhufmann bhufmann requested a review from PatrickTasse March 17, 2021 19:52
bhufmann added a commit to bhufmann/vscode-trace-extension that referenced this pull request Mar 23, 2021
- This change includes updates to propagate signals between webviews.
This will allow views to be updated when one view changes. For example,
when the user selects an open trace in the "Traces" view, the selection
is sent to the various react-components. The "Available Views" view
will use that signal as trigger to fetch the available views from
the remote server.

- Remove analysis and traces tree view from the extension. They are
replaced by the theia-trace-extension react components and
corresponding webviews.

- The classes AnalysisTree and TracesTree is still in the code base to
allow opening traces from the file system using the context sensitive
menu and "Open in Trace Viewer" command. This will have to be clean-up
in a later patch (+ using a cancelable progress monitor).

- The patch also includes a fix to make source-map work when debugging
the webview apps.

Needs eclipse-cdt-cloud/theia-trace-extension#295

Fixes eclipse-cdt-cloud#4
Fixes eclipse-cdt-cloud#7
Fixes eclipse-cdt-cloud#11
Fixes eclipse-cdt-cloud#16

Signed-off-by: Bernd Hufmann <Bernd.Hufmann@ericsson.com>
bhufmann added a commit to bhufmann/vscode-trace-extension that referenced this pull request Mar 26, 2021
- This change includes updates to propagate signals between webviews.
This will allow views to be updated when one view changes. For example,
when the user selects an open trace in the "Traces" view, the selection
is sent to the various react-components. The "Available Views" view
will use that signal as trigger to fetch the available views from
the remote server.

- Remove analysis and traces tree view from the extension. They are
replaced by the theia-trace-extension react components and
corresponding webviews.

- The classes AnalysisTree and TracesTree is still in the code base to
allow opening traces from the file system using the context sensitive
menu and "Open in Trace Viewer" command. This will have to be clean-up
in a later patch (+ using a cancelable progress monitor).

- The patch also includes a fix to make source-map work when debugging
the webview apps.

Needs eclipse-cdt-cloud/theia-trace-extension#295

Fixes eclipse-cdt-cloud#4
Fixes eclipse-cdt-cloud#7
Fixes eclipse-cdt-cloud#11
Fixes eclipse-cdt-cloud#16

Signed-off-by: Bernd Hufmann <Bernd.Hufmann@ericsson.com>
- Remove direct dependencies between class
- Streamline signal sending and receiving
- Move signal manager in own package
- Make sure removeListener actually removes listener

This decoupling will make it easier to create standalone react
components for trace-explorer-opened-traces-widget.tsx and
trace-explorer-analysis-widget.tsx that can be re-used outside the
Theia framework.

Signed-off-by: Bernd Hufmann <Bernd.Hufmann@ericsson.com>
This new react component ReactOpenTracesWidget will be part of the
packages/react-components. It can be used in other web applications
independently from Theia.

Signed-off-by: Bernd Hufmann <Bernd.Hufmann@ericsson.com>
This new react component ReactAvailableViewsWidget will be part of the
packages/react-components. It can be used in other web applications
independently from Theia.

It listens to the AVAILABLE_OUTPUTS_CHANGED Signal to update the widget.

Signed-off-by: Bernd Hufmann <Bernd.Hufmann@ericsson.com>
With this change the ReactAnalysisWidget is responsible to
get the available analyses from the remote instead of the
ReactOpenedTracesWidget.

The ReactOpenTracesWidget was updated to send EXPERIMENT_SELECTED signal
when to indicate that experiment selection was changed.
ReactAnalysisWidget will update upon reception of the
EXPERIMENT_SELECTED signal.

Signed-off-by: Bernd Hufmann <Bernd.Hufmann@ericsson.com>
@bhufmann bhufmann merged commit 23a583d into eclipse-cdt-cloud:master Mar 29, 2021
@bhufmann bhufmann deleted the decouple-trace-explorer branch March 29, 2021 13:48
bhufmann added a commit to bhufmann/vscode-trace-extension that referenced this pull request Mar 31, 2021
- This change includes updates to propagate signals between webviews.
This will allow views to be updated when one view changes. For example,
when the user selects an open trace in the "Traces" view, the selection
is sent to the various react-components. The "Available Views" view
will use that signal as trigger to fetch the available views from
the remote server.

- Remove analysis and traces tree view from the extension. They are
replaced by the theia-trace-extension react components and
corresponding webviews.

- The classes AnalysisTree and TracesTree is still in the code base to
allow opening traces from the file system using the context sensitive
menu and "Open in Trace Viewer" command. This will have to be clean-up
in a later patch (+ using a cancelable progress monitor).

- The patch also includes a fix to make source-map work when debugging
the webview apps.

Needs eclipse-cdt-cloud/theia-trace-extension#295

Fixes eclipse-cdt-cloud#4
Fixes eclipse-cdt-cloud#7
Fixes eclipse-cdt-cloud#11
Fixes eclipse-cdt-cloud#16

Signed-off-by: Bernd Hufmann <Bernd.Hufmann@ericsson.com>
bhufmann added a commit to bhufmann/vscode-trace-extension that referenced this pull request Mar 31, 2021
- This change includes updates to propagate signals between webviews.
This will allow views to be updated when one view changes. For example,
when the user selects an open trace in the "Traces" view, the selection
is sent to the various react-components. The "Available Views" view
will use that signal as trigger to fetch the available views from
the remote server.

- Remove analysis and traces tree view from the extension. They are
replaced by the theia-trace-extension react components and
corresponding webviews.

- The classes AnalysisTree and TracesTree is still in the code base to
allow opening traces from the file system using the context sensitive
menu and "Open in Trace Viewer" command. This will have to be clean-up
in a later patch (+ using a cancelable progress monitor).

- The patch also includes a fix to make source-map work when debugging
the webview apps.

Needs eclipse-cdt-cloud/theia-trace-extension#295

Fixes eclipse-cdt-cloud#7
Fixes eclipse-cdt-cloud#11
Fixes eclipse-cdt-cloud#16

Signed-off-by: Bernd Hufmann <Bernd.Hufmann@ericsson.com>
bhufmann added a commit to bhufmann/vscode-trace-extension that referenced this pull request Mar 31, 2021
- This change includes updates to propagate signals between webviews.
This will allow views to be updated when one view changes. For example,
when the user selects an open trace in the "Traces" view, the selection
is sent to the various react-components. The "Available Views" view
will use that signal as trigger to fetch the available views from
the remote server.

- Remove analysis and traces tree view from the extension. They are
replaced by the theia-trace-extension react components and
corresponding webviews.

- The classes AnalysisTree and TracesTree is still in the code base to
allow opening traces from the file system using the context sensitive
menu and "Open in Trace Viewer" command. This will have to be clean-up
in a later patch (+ using a cancelable progress monitor).

- The patch also includes a fix to make source-map work when debugging
the webview apps.

Needs eclipse-cdt-cloud/theia-trace-extension#295

Fixes eclipse-cdt-cloud#7
Fixes eclipse-cdt-cloud#11
Fixes eclipse-cdt-cloud#16

Signed-off-by: Bernd Hufmann <Bernd.Hufmann@ericsson.com>
bhufmann added a commit to eclipse-cdt-cloud/vscode-trace-extension that referenced this pull request Apr 5, 2021
- This change includes updates to propagate signals between webviews.
This will allow views to be updated when one view changes. For example,
when the user selects an open trace in the "Traces" view, the selection
is sent to the various react-components. The "Available Views" view
will use that signal as trigger to fetch the available views from
the remote server.

- Remove analysis and traces tree view from the extension. They are
replaced by the theia-trace-extension react components and
corresponding webviews.

- The classes AnalysisTree and TracesTree is still in the code base to
allow opening traces from the file system using the context sensitive
menu and "Open in Trace Viewer" command. This will have to be clean-up
in a later patch (+ using a cancelable progress monitor).

- The patch also includes a fix to make source-map work when debugging
the webview apps.

Needs eclipse-cdt-cloud/theia-trace-extension#295

Fixes #7
Fixes #11
Fixes #16

Signed-off-by: Bernd Hufmann <Bernd.Hufmann@ericsson.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants