Create a new 'Open in Trace Viewer' command, add it to navigator context menu #1119
Conversation
marcdumais-work
changed the title
marcdumais-work
force-pushed
the
new-open-with-command
branch
2 times, most recently
from
f20f42c
to
add7725
Compare
marcdumais-work
force-pushed
the
new-open-with-command
branch
from
add7725
to
622a5a6
Compare
theia-extensions/viewer-prototype/src/browser/trace-explorer/trace-explorer-commands.ts
Show resolved
Hide resolved
marcdumais-work
force-pushed
the
new-open-with-command
branch
from
622a5a6
to
1d76e20
Compare
marcdumais-work
changed the title
README.md
Outdated
| @@ -466,7 +466,7 @@ The code in this repository is licensed under `MIT` (see root `LICENSE`), except | |||
| [image-icon]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-icon.png | |||
| [image-open-browser]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-open-browser.png | |||
| [image-open-view]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-open-view.gif | |||
| [image-open-with]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-open-with-trace-viewer.gif | |||
| [image-open-with-trace-viewer]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-open-with-trace-viewer.gif | |||
There was a problem hiding this comment.
Github caches images. So, even if you change the source file of the image it will still show the old file on the webpage. So, what I usually do is to have a version number on the file, include that new file and delete the old one with the old name (see below for an example).
There was a problem hiding this comment.
I've seen your comment below, about being ok with "open in" - I'll restore that version and it will fix this comment, since the .gif file is named differently.
| export const OPEN_WITH_TRACE_VIEWER: Command = { | ||
| id: 'trace-explorer:open-with-trace-viewer', | ||
| label: 'Open with Trace Viewer' | ||
| }; |
There was a problem hiding this comment.
Not sure where to add the comment. Could you please change the priority of the menu entry of that command so that it's lower in the list, preferably last. We did the same change in the vscode-trace-extension PR 249 after receiving user feedback that it's too intrusive (i.e. too high up with the common open commands). See image. Not sure how to do that in Theia but it should be possible.
There was a problem hiding this comment.
change the priority of the menu entry of that command so that it's lower in the list, preferably last
easily done
[use "open in trace viewer"] we should also change it in the vscode-trace-extension
Created an issue, that we can schedule as we see fit: eclipse-cdt-cloud/vscode-trace-extension#275
There was a problem hiding this comment.
I pushed a new version that should address the comments. The context menu entry should be the last item in the "Navigation" section (top part) of the menu. We can't control the order of new entries that might get added by others, but with the vanilla entries, added by Theia, we should be consistently at the end of the section.
There was a problem hiding this comment.
One question I have: are you bugged by the constant and "id" using "OPEN_WITH..." / "...open-with..." but presenting the command as "open in..."?:
There was a problem hiding this comment.
If so, I can align everything to use "open in"
There was a problem hiding this comment.
It's now or never because after we release it the command could be triggered by a third-party extension.
There was a problem hiding this comment.
Good point - let's do it then
There was a problem hiding this comment.
done, new version pushed
marcdumais-work
changed the title
marcdumais-work
force-pushed
the
new-open-with-command
branch
3 times, most recently
from
6747e89
to
1beaa17
Compare
Signed-off-by: Marc Dumais <[email protected]>
…ext menu Starting around Theia v1.50.0, the "open-with" service no longer permitted to open folders from the navigator context menu (only worked for files). It was possible to make it work again, but there were undesirable side-effects. So instead, we decided to register our own command, "Open in Trace Viewer", and add it to the navigator's context menu at the first level. This is very similar to what we have on the `Trace Viewer for VSCode` extension. Also updated the README to reflect how a trace can now be be opened from the File Explorer, and recorded a new .gif showing it. Signed-off-by: Marc Dumais <[email protected]>
marcdumais-work
force-pushed
the
new-open-with-command
branch
from
1beaa17
to
85775c9
Compare
Starting around Theia v1.50.0, the "open-with" service no longer permitted to open folders from the navigator context menu (only worked for files). It was possible to make it work again, but there were undesirable side-effects. So instead, we decided to register our own command, "Open with Trace Viewer", and add it to the navigator's context menu at the first level. This is very similar to what we have on the
Trace Viewer for VSCodeextension.Also updated the README to reflect how a trace can now be be opened from the File Explorer, and recorded a new .gif showing it (will only be visible from the README after the PR is merged).
Note: this PR is currently on top of the upgrade to Theia 1.52.0, that has its own PR. We can merge that one first, or I can re-organize the commits if needed.