Support opening trace files through a dialog #1054
Conversation
ngondalia
force-pushed
the
handleOpenTraceFiles
branch
from
fa62b38 to
257eeb7
Compare
openbuttons.mp4 |
There was a problem hiding this comment.
Overall, I like this approach of allowing the extension to control which option gets rendered. The UX would now have to explicitly state 'Open Trace File' or 'Open Trace Folder'
- In
trace-viewer-commands.ts, could you rename the label of the OpenTraceCommand to 'Open Trace Folder'? This is so that if a user decides to open a trace through the command palette they know what type of traces they can access. You could even add a command for 'Open Trace File' in this case.
theia-extensions/viewer-prototype/src/browser/trace-viewer/trace-viewer-toolbar-commands.ts
Outdated
Show resolved
Hide resolved
...a-extensions/viewer-prototype/src/browser/trace-viewer/trace-viewer-toolbar-contribution.tsx
Outdated
Show resolved
Hide resolved
ngondalia
force-pushed
the
handleOpenTraceFiles
branch
from
257eeb7 to
1ef8e2f
Compare
Thanks for reviewing! I have addressed your comments. |
| if (healthResponse && healthResponse.isOk() && healthResponse.getModel()?.status === 'UP') { | ||
| this.openDialog(rootPath); | ||
| this.openDialog(rootPath, selectFiles); |
There was a problem hiding this comment.
I think we should change the title of the dialog. Right now the title is Open Trace for both cases. It might not be clear for the user. Maybe change the title to Open Trace File/Open Trace Folder or Select Trace File to Open/Select Trace Folder to Open.
| <div className="placeholder-open-workspace-button-container"> | ||
| <button | ||
| className="plcaeholder-open-workspace-button" | ||
| title="Select a trace to open" |
There was a problem hiding this comment.
This should be different for each case to be clear to the user, maybe 'Select a single trace file to open' for file traces and 'Select a trace folder to open`
There was a problem hiding this comment.
Ok - I will update it in the next patch!
ngondalia
force-pushed
the
handleOpenTraceFiles
branch
from
1ef8e2f to
1a67a62
Compare
This commit adds another button for opening trace files through a dialog. Futhermore, the theia-trace-explorer-placeholder-widget can be configured to render either one or both buttons (Open Trace folder, Open Trace File). Fixes eclipse-cdt-cloud#1053 Signed-off-by: Neel Gondalia <ngondalia@blackberry.com>
ngondalia
force-pushed
the
handleOpenTraceFiles
branch
from
1a67a62 to
32f0abd
Compare
|
@hriday-panchasara could you please update your review? |
This commit adds another button for opening trace files through a dialog. Futhermore, the theia-trace-explorer-placeholder-widget can be configured to render either one or both buttons (Open Trace folder, Open Trace File).
Fixes #1053
Signed-off-by: Neel Gondalia ngondalia@blackberry.com