feat(getting-started): support SVG rendering and interactive command links in walkthroughs - #17954
feat(getting-started): support SVG rendering and interactive command links in walkthroughs#17954PrathamS1 wants to merge 2 commits into
Conversation
…kthroughs Signed-off-by: Pratham <pratham.singh045@gmail.com>
ndoschek
left a comment
ndoschek
left a comment
There was a problem hiding this comment.
Hi @PrathamS1, thanks for tackling this!
Inline SVG rendering with clickable command links is the right approach, and I appreciate that it comes with tests. But the PR does considerably more than its title says, and the extra parts aren't precise enough to go in as they are. I'd prefer to see it reduced to the walkthrough SVG feature only.
Could you also please use the complete pull request template for the PR description, since we use sections like the breaking change section for the automated changelog generation.
Thanks!
There was a problem hiding this comment.
I would prefer to drop the .gs-walkthrough-media-image sizing changes:
It resizes every existing walkthrough image, and it works against the goal: fixed max-width: 460px isn't responsive, and since that rule comes after the shared one it overrides max-width: 100%, so images are no longer clamped to their container and overflow whenever the panel is narrower than 460px. The same caps on .gs-walkthrough-media-svg svg are better placed but still arbitrary.
| @@ -94,6 +94,10 @@ export class ColorApplicationContribution implements FrontendApplicationContribu | |||
| const { name, value } = variable; | |||
| documentElement.style.setProperty(name, value); | |||
| this.toUpdate.push(Disposable.create(() => documentElement.style.removeProperty(name))); | |||
|
|
|||
| const vscodeName = this.colors.toCssVariableName(id, 'vscode'); | |||
There was a problem hiding this comment.
This --vscode-* color aliasing should be dropped here and be worked on in a separate PR IMO.
This is already tracked in #17412, but it's an application-wide @theia/core change that shouldn't come along with another feature IMO.
As this is implemented now, it will create ~1000 extra inline properties on documentElement rewritten on every theme change as mentioned in the issue. It should also drop the ad-hoc aliases it makes obsolete, and be smoke-tested outside .monaco-editor / .monaco-diff-editor / .monaco-component.
| commands.registerCommand(VSXExtensionsCommands.SHOW_LANGUAGE_EXTENSIONS, { | ||
| execute: () => this.showLanguageExtensions() | ||
| }); | ||
|
|
||
| commands.registerCommand(VSXExtensionsCommands.SEARCH, { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| execute: (query?: any) => this.searchExtensions(query) | ||
| }); | ||
|
|
||
| commands.registerCommand(VSXExtensionsCommands.SHOW_EXTENSIONS_WITH_IDS, { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| execute: (extensionIds: any) => this.showExtensionsWithIds(extensionIds) | ||
| }); | ||
|
|
||
| if (typeof commands.registerAlias === 'function') { | ||
| commands.registerAlias('workbench.view.extensions', VSXExtensionsCommands.SHOW_INSTALLED.id); | ||
| commands.registerAlias('workbench.extensions.action.showInstalledExtensions', VSXExtensionsCommands.SHOW_INSTALLED.id); | ||
| commands.registerAlias('workbench.extensions.action.showRecommendedExtensions', VSXExtensionsCommands.SHOW_RECOMMENDATIONS.id); | ||
| commands.registerAlias('workbench.extensions.action.listBuiltInExtensions', VSXExtensionsCommands.SHOW_BUILTINS.id); | ||
| } | ||
|
|
There was a problem hiding this comment.
Please also drop the vsx-registry commands:
I see where they come from, the one link in plugins/getting-started-sample/media/image.svg is command:workbench.extensions.action.showLanguageExtensions, but two of the three don't work: Our VSXExtensionsSearchModel only understands @installed, @builtin, @recommended and registered type tokens, so @category:"programming languages" and @id:<id> become literal free-text searches which will not show any results.
Overall just to get this sample extension to work we should not introduce these commands, as this extension is not very likely to be installed by regular Theia users.
There was a problem hiding this comment.
Thank you for the review and guidance @ndoschek! I have updated the PR to focus strictly on the walkthrough SVG rendering and link execution in @theia/getting-started.
Summary of updates in the latest commit:
- Reverted @theia/core color aliasing (referenced Set --vscode- CSS color variables alongside --theia- for VS Code extension compatibility #17412 in Follow-ups). noting that SVGs relying on var(--vscode-foreground) without fallbacks will display their text once Set --vscode- CSS color variables alongside --theia- for VS Code extension compatibility #17412 is resolved).
- Reverted @theia/vsx-registry command additions.
- Adjusted CSS to keep max-width: 100% on images and SVGs without arbitrary fixed-pixel caps.
- Updated the PR description with the complete PR template.
github-project-automation
Bot
moved this from Waiting on reviewers
to Waiting on author
in PR Backlog
…s review feedback Signed-off-by: Pratham <pratham.singh045@gmail.com>
2 participants
What it does
Fixes #17949: Adds support for rendering inline SVG media in walkthrough steps and executing interactive command links embedded within SVGs.
WalkthroughMediaSvgusingDOMPurifyto securely render inline SVGs from extensions while retaining<use>,<a>, andxlink:hrefcommand links.<a>/xlink:hrefelements, forwarding clicked URLs toWalkthroughService.handleLinkClickfor command execution and step progress tracking..gs-walkthrough-media-svg svgwithmax-width: 100%; height: auto;to adapt cleanly across different panel widths without overriding standard walkthrough image constraints.How to test
Install the sample walkthrough extension (or place it under
plugins/).Start the browser application and open the walkthrough: Welcome -> Walkthrough: Sample.
Select the Use SVG's step:
- Verify the SVG renders inline.
- Click the language section in the SVG and verify that click events are dispatched and routed to
WalkthroughService.handleLinkClick.Check standard walkthroughs with raster images (e.g. Get Started with Java Development):
Follow-ups
--vscode-*CSS variable resolution (used by theme-adaptive SVGs without fallbacks) is tracked separately in Set --vscode- CSS color variables alongside --theia- for VS Code extension compatibility #17412. (Note: SVGs using static/fallback colors, like in the Java walkthrough, render completely. Theme-adaptive SVGs relying exclusively onvar(--vscode-foreground)without a fallback will show full text once Set --vscode- CSS color variables alongside --theia- for VS Code extension compatibility #17412 lands).Breaking changes
Review checklist
nlsservice (for details, please see the Internationalization/Localization section in the Coding Guidelines)Reminder for reviewers