Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

[Monaco] Make minimap toggleable #33742

Merged
merged 3 commits into from
Jan 14, 2025
Merged

[Monaco] Make minimap toggleable #33742

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cf7ac64
Reflect changes from #35122
PesBandi Dec 6, 2024
4c6be70
Update index.html
PesBandi Dec 7, 2024
ae62249
Merge branch 'main' into pr33742
jaimecbernardo Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 32 additions & 19 deletions src/Monaco/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,19 @@
// `theme` can be "vs" for light theme or "vs-dark" for dark theme
// `lang` is the language of the file
// `wrap` if the editor is wrapping or not
// `minimap` if the minimap is shown
// `contextMenu` whether to use the Monaco context menu. The built-in context menu
// doesn't work in Peek, so we set this to false and create a custom one

var theme = ("[[PT_THEME]]" == "dark") ? "vs-dark" : "vs";
var wrap = [[PT_WRAP]];
var minimap = [[PT_MINIMAP]];
var stickyScroll = [[PT_STICKY_SCROLL]];
var fontSize = [[PT_FONT_SIZE]];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these equivalent in JavaScript? Why the change from from ternary to this?


var lang = "[[PT_LANG]]";
var wrap = ([[PT_WRAP]] == 1) ? true : false;

var base64code = "[[PT_CODE]]";

var stickyScroll = ([[PT_STICKY_SCROLL]] == 1) ? true : false;

var fontSize = [[PT_FONT_SIZE]];
var contextMenu = ([[PT_CONTEXTMENU]] == 1) ? true : false;
var contextMenu = [[PT_CONTEXTMENU]];

var editor;

Expand All @@ -29,12 +31,13 @@
}).join(''));

function runToggleTextWrapCommand() {
if (wrap) {
editor.updateOptions({ wordWrap: 'off' })
} else {
editor.updateOptions({ wordWrap: 'on' })
}
wrap = !wrap;
editor.updateOptions({ wordWrap: wrap ? 'on' : 'off' });
}

function runToggleMinimap() {
minimap = !minimap;
editor.updateOptions({minimap: {enabled: minimap}});
}

function runCopyCommand() {
Expand Down Expand Up @@ -99,8 +102,8 @@
language: lang, // Sets language of the code
readOnly: true, // Sets to readonly
theme: 'theme', // Sets editor theme
minimap: { enabled: false }, // Disables minimap
lineNumbersMinChars: '3', // Width of the line numbers
minimap: { enabled: minimap }, // Controls if minimap is shown
lineNumbersMinChars: 3, // Width of the line numbers
contextmenu: contextMenu,
scrollbar: {
// Deactivate shadows
Expand Down Expand Up @@ -135,10 +138,20 @@
contextMenuOrder: 100,

// Method that will be executed when the action is triggered.
// @param editor The editor instance is passed in as a convenience
run: function (ed) {
runToggleTextWrapCommand();
}
run: runToggleTextWrapCommand
});

editor.addAction({
id: 'toggle-minimap',

label: 'Toggle minimap',

contextMenuGroupId: 'cutcopypaste',

contextMenuOrder: 100,

// Method that will be executed when the action is triggered.
run: runToggleMinimap
});

onContextMenu();
Expand Down Expand Up @@ -166,4 +179,4 @@
}
</script>
</body>
</html>
</html>
9 changes: 5 additions & 4 deletions src/modules/peek/Peek.FilePreviewer/Controls/BrowserControl.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,11 @@ MenuItem CreateCommandMenuItem(string resourceId, string commandName)
// When using Monaco, we show menu items that call the appropriate JS functions -
// WebView2 isn't able to show a "Copy" menu item of its own.
return [
CreateCommandMenuItem("ContextMenu_Copy", "runCopyCommand"),
new Separator(),
CreateCommandMenuItem("ContextMenu_ToggleTextWrapping", "runToggleTextWrapCommand"),
];
CreateCommandMenuItem("ContextMenu_Copy", "runCopyCommand"),
new Separator(),
CreateCommandMenuItem("ContextMenu_ToggleTextWrapping", "runToggleTextWrapCommand"),
CreateCommandMenuItem("ContextMenu_ToggleMinimap", "runToggleMinimap")
];
}
else
{
Expand Down
2 changes: 2 additions & 0 deletions src/modules/peek/Peek.FilePreviewer/Models/IPreviewSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,7 @@ public interface IPreviewSettings
public int SourceCodeFontSize { get; }

public bool SourceCodeStickyScroll { get; }

public bool SourceCodeMinimap { get; }
}
}
4 changes: 4 additions & 0 deletions src/modules/peek/Peek.FilePreviewer/Models/PreviewSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,16 @@ public class PreviewSettings : IPreviewSettings

public bool SourceCodeStickyScroll { get; private set; }

public bool SourceCodeMinimap { get; private set; }

public PreviewSettings()
{
_settingsUtils = new SettingsUtils();
SourceCodeWrapText = false;
SourceCodeTryFormat = false;
SourceCodeFontSize = 14;
SourceCodeStickyScroll = true;
SourceCodeMinimap = false;

LoadSettingsFromJson();

Expand Down Expand Up @@ -70,6 +73,7 @@ private void LoadSettingsFromJson()
SourceCodeTryFormat = settings.SourceCodeTryFormat.Value;
SourceCodeFontSize = settings.SourceCodeFontSize.Value;
SourceCodeStickyScroll = settings.SourceCodeStickyScroll.Value;
SourceCodeMinimap = settings.SourceCodeMinimap.Value;
}

retry = false;
Expand Down
13 changes: 7 additions & 6 deletions src/modules/peek/Peek.FilePreviewer/Previewers/WebBrowserPreviewer/Helpers/MonacoHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ public static HashSet<string> GetExtensions()
/// <summary>
/// Prepares temp html for the previewing
/// </summary>
public static string PreviewTempFile(string fileText, string extension, string tempFolder, bool tryFormat, bool wrapText, bool stickyScroll, int fontSize)
public static string PreviewTempFile(string fileText, string extension, string tempFolder, bool tryFormat, bool wrapText, bool stickyScroll, int fontSize, bool minimap)
{
// TODO: check if file is too big, add MaxFileSize to settings
return InitializeIndexFileAndSelectedFile(fileText, extension, tempFolder, tryFormat, wrapText, stickyScroll, fontSize);
return InitializeIndexFileAndSelectedFile(fileText, extension, tempFolder, tryFormat, wrapText, stickyScroll, fontSize, minimap);
}

private static string InitializeIndexFileAndSelectedFile(string fileContent, string extension, string tempFolder, bool tryFormat, bool wrapText, bool stickyScroll, int fontSize)
private static string InitializeIndexFileAndSelectedFile(string fileContent, string extension, string tempFolder, bool tryFormat, bool wrapText, bool stickyScroll, int fontSize, bool minimap)
{
string vsCodeLangSet = Microsoft.PowerToys.FilePreviewCommon.MonacoHelper.GetLanguage(extension);

Expand All @@ -81,13 +81,14 @@ private static string InitializeIndexFileAndSelectedFile(string fileContent, str
string html = Microsoft.PowerToys.FilePreviewCommon.MonacoHelper.ReadIndexHtml();

html = html.Replace("[[PT_LANG]]", vsCodeLangSet, StringComparison.InvariantCulture);
html = html.Replace("[[PT_WRAP]]", wrapText ? "1" : "0", StringComparison.InvariantCulture);
html = html.Replace("[[PT_CONTEXTMENU]]", "0", StringComparison.InvariantCulture);
html = html.Replace("[[PT_STICKY_SCROLL]]", stickyScroll ? "1" : "0", StringComparison.InvariantCulture);
html = html.Replace("[[PT_WRAP]]", wrapText ? "true" : "false", StringComparison.InvariantCulture);
html = html.Replace("[[PT_CONTEXTMENU]]", "false", StringComparison.InvariantCulture);
html = html.Replace("[[PT_STICKY_SCROLL]]", stickyScroll ? "true" : "false", StringComparison.InvariantCulture);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Once again here, why the change of logic? I'm not saying it's wrong, but I'd like to understand the reasoning behind it.

html = html.Replace("[[PT_THEME]]", theme, StringComparison.InvariantCulture);
html = html.Replace("[[PT_FONT_SIZE]]", fontSize.ToString(CultureInfo.InvariantCulture), StringComparison.InvariantCulture);
html = html.Replace("[[PT_CODE]]", base64FileCode, StringComparison.InvariantCulture);
html = html.Replace("[[PT_URL]]", Microsoft.PowerToys.FilePreviewCommon.MonacoHelper.VirtualHostName, StringComparison.InvariantCulture);
html = html.Replace("[[PT_MINIMAP]]", minimap ? "true" : "false", StringComparison.InvariantCulture);

string filename = tempFolder + "\\" + Guid.NewGuid().ToString() + ".html";
File.WriteAllText(filename, html);
Expand Down
2 changes: 1 addition & 1 deletion src/modules/peek/Peek.FilePreviewer/Previewers/WebBrowserPreviewer/WebBrowserPreviewer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ await Dispatcher.RunOnUiThread(async () =>
if (useMonaco)
{
var raw = await ReadHelper.Read(File.Path.ToString());
Preview = new Uri(MonacoHelper.PreviewTempFile(raw, File.Extension, TempFolderPath.Path, _previewSettings.SourceCodeTryFormat, _previewSettings.SourceCodeWrapText, _previewSettings.SourceCodeStickyScroll, _previewSettings.SourceCodeFontSize));
Preview = new Uri(MonacoHelper.PreviewTempFile(raw, File.Extension, TempFolderPath.Path, _previewSettings.SourceCodeTryFormat, _previewSettings.SourceCodeWrapText, _previewSettings.SourceCodeStickyScroll, _previewSettings.SourceCodeFontSize, _previewSettings.SourceCodeMinimap));
}
else if (isMarkdown)
{
Expand Down
3 changes: 3 additions & 0 deletions src/modules/peek/Peek.UI/Strings/en-us/Resources.resw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,4 +326,7 @@
<value>Toggle text wrapping</value>
<comment>Toggle whether text in pane is word-wrapped</comment>
</data>
<data name="ContextMenu_ToggleMinimap" xml:space="preserve">
<value>Toggle minimap</value>
</data>
</root>
7 changes: 4 additions & 3 deletions src/modules/previewpane/MonacoPreviewHandler/MonacoPreviewHandlerControl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,13 +396,14 @@ private void InitializeIndexFileAndSelectedFile(string filePath)
// prepping index html to load in
_html = FilePreviewCommon.MonacoHelper.ReadIndexHtml();
_html = _html.Replace("[[PT_LANG]]", _vsCodeLangSet, StringComparison.InvariantCulture);
_html = _html.Replace("[[PT_WRAP]]", _settings.Wrap ? "1" : "0", StringComparison.InvariantCulture);
_html = _html.Replace("[[PT_CONTEXTMENU]]", "1", StringComparison.InvariantCulture);
_html = _html.Replace("[[PT_WRAP]]", _settings.Wrap ? "true" : "false", StringComparison.InvariantCulture);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Logic changed here, as asked on other comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @jaimecbernardo, thanks for the review.
I made these changes to simplify the logic. I think the original was unnecessarily complicated. I don't see any reason to do (1 == 1) ? true : false. First of all, the ternary is completely unneeded, since (1 == 1) already returns a boolean. I also don't understand why we do this comparison in the first place when we can just replace [[PT_SOMETHING]] with true/false and require no extra logic in the javascript. It does the exact same thing just cleaner (IMO). Let me know if I'm missing something.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the info. I think that makes sense. I don't think that brings any issue. I'll give it a test soon to check if everything's OK ;)
Thanks for opening this PR.

_html = _html.Replace("[[PT_CONTEXTMENU]]", "true", StringComparison.InvariantCulture);
_html = _html.Replace("[[PT_THEME]]", Settings.GetTheme(), StringComparison.InvariantCulture);
_html = _html.Replace("[[PT_STICKY_SCROLL]]", _settings.StickyScroll ? "1" : "0", StringComparison.InvariantCulture);
_html = _html.Replace("[[PT_STICKY_SCROLL]]", _settings.StickyScroll ? "true" : "false", StringComparison.InvariantCulture);
_html = _html.Replace("[[PT_FONT_SIZE]]", _settings.FontSize.ToString(CultureInfo.InvariantCulture), StringComparison.InvariantCulture);
_html = _html.Replace("[[PT_CODE]]", _base64FileCode, StringComparison.InvariantCulture);
_html = _html.Replace("[[PT_URL]]", FilePreviewCommon.MonacoHelper.VirtualHostName, StringComparison.InvariantCulture);
_html = _html.Replace("[[PT_MINIMAP]]", _settings.Minimap ? "true" : "false", StringComparison.InvariantCulture);
}

private async void DownloadLink_Click(object sender, EventArgs e)
Expand Down
20 changes: 20 additions & 0 deletions src/modules/previewpane/MonacoPreviewHandler/Settings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,26 @@ public bool StickyScroll
}
}

/// <summary>
/// Gets a value indicating whether the minimap should be enabled. Set by PT settings.
/// </summary>
public bool Minimap
{
get
{
try
{
return moduleSettings.GetSettings<PowerPreviewSettings>(PowerPreviewSettings.ModuleName).Properties.MonacoPreviewMinimap;
}
catch (FileNotFoundException)
{
// Couldn't read the settings
// Assume default of false
return false;
}
}
}

/// <summary>
/// Gets the color of the window background.
/// </summary>
Expand Down
3 changes: 3 additions & 0 deletions src/settings-ui/Settings.UI.Library/PeekPreviewSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,15 @@ public class PeekPreviewSettings : ISettingsConfig

public BoolProperty SourceCodeStickyScroll { get; set; }

public BoolProperty SourceCodeMinimap { get; set; }

public PeekPreviewSettings()
{
SourceCodeWrapText = new BoolProperty(false);
SourceCodeTryFormat = new BoolProperty(false);
SourceCodeFontSize = new IntProperty(14);
SourceCodeStickyScroll = new BoolProperty(true);
SourceCodeMinimap = new BoolProperty(false);
}

public string ToJsonString()
Expand Down
17 changes: 17 additions & 0 deletions src/settings-ui/Settings.UI.Library/PowerPreviewProperties.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,23 @@ public bool MonacoPreviewStickyScroll
}
}

private bool monacoPreviewMinimap;

[JsonPropertyName("monaco-previewer-minimap")]
[JsonConverter(typeof(BoolPropertyJsonConverter))]
public bool MonacoPreviewMinimap
{
get => monacoPreviewMinimap;
set
{
if (value != monacoPreviewMinimap)
{
LogTelemetryEvent(value);
monacoPreviewMinimap = value;
}
}
}

private bool enablePdfPreview;

[JsonPropertyName("pdf-previewer-toggle-setting")]
Expand Down
3 changes: 3 additions & 0 deletions src/settings-ui/Settings.UI/SettingsXAML/Views/PeekPage.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@
<tkcontrols:SettingsCard ContentAlignment="Left" IsEnabled="{x:Bind ViewModel.IsEnabled, Mode=OneWay}">
<CheckBox x:Uid="Peek_SourceCode_StickyScroll" IsChecked="{x:Bind ViewModel.SourceCodeStickyScroll, Mode=TwoWay}" />
</tkcontrols:SettingsCard>
<tkcontrols:SettingsCard ContentAlignment="Left" IsEnabled="{x:Bind ViewModel.IsEnabled, Mode=OneWay}">
<CheckBox x:Uid="Peek_SourceCode_Minimap" IsChecked="{x:Bind ViewModel.SourceCodeMinimap, Mode=TwoWay}" />
</tkcontrols:SettingsCard>
</tkcontrols:SettingsExpander.Items>
</tkcontrols:SettingsExpander>
</controls:SettingsGroup>
Expand Down
6 changes: 6 additions & 0 deletions src/settings-ui/Settings.UI/SettingsXAML/Views/PowerPreviewPage.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@
IsChecked="{x:Bind ViewModel.MonacoPreviewStickyScroll, Mode=TwoWay}"
IsEnabled="{x:Bind ViewModel.MonacoRenderIsEnabled, Mode=OneWay}" />
</tkcontrols:SettingsCard>
<tkcontrols:SettingsCard ContentAlignment="Left" IsEnabled="{x:Bind ViewModel.MonacoRenderIsEnabled, Mode=OneWay}">
<CheckBox
x:Uid="FileExplorerPreview_ToggleSwitch_Monaco_Minimap"
IsChecked="{x:Bind ViewModel.MonacoPreviewMinimap, Mode=TwoWay}"
IsEnabled="{x:Bind ViewModel.MonacoRenderIsEnabled, Mode=OneWay}" />
</tkcontrols:SettingsCard>
</tkcontrols:SettingsExpander.Items>
</tkcontrols:SettingsExpander>

Expand Down
6 changes: 6 additions & 0 deletions src/settings-ui/Settings.UI/Strings/en-us/Resources.resw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4393,6 +4393,9 @@ Activate by holding the key for the character you want to add an accent to, then
<data name="Peek_SourceCode_WrapText.Content" xml:space="preserve">
<value>Wrap text</value>
</data>
<data name="Peek_SourceCode_Minimap.Content" xml:space="preserve">
<value>Show minimap</value>
</data>
<data name="ShowPluginsOverview_All.Content" xml:space="preserve">
<value>All</value>
</data>
Expand Down Expand Up @@ -4439,6 +4442,9 @@ Activate by holding the key for the character you want to add an accent to, then
<data name="Peek_SourceCode_StickyScroll.Content" xml:space="preserve">
<value>Enable sticky scroll</value>
</data>
<data name="FileExplorerPreview_ToggleSwitch_Monaco_Minimap.Content" xml:space="preserve">
<value>Show minimap</value>
</data>
<data name="PrivacyLink.Text" xml:space="preserve">
<value>OpenAI Privacy</value>
</data>
Expand Down
14 changes: 14 additions & 0 deletions src/settings-ui/Settings.UI/ViewModels/PeekViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,20 @@ public bool SourceCodeStickyScroll
}
}

public bool SourceCodeMinimap
{
get => _peekPreviewSettings.SourceCodeMinimap.Value;
set
{
if (_peekPreviewSettings.SourceCodeMinimap.Value != value)
{
_peekPreviewSettings.SourceCodeMinimap.Value = value;
OnPropertyChanged(nameof(SourceCodeMinimap));
SavePreviewSettings();
}
}
}

private void NotifySettingsChanged()
{
// Using InvariantCulture as this is an IPC message
Expand Down
16 changes: 16 additions & 0 deletions src/settings-ui/Settings.UI/ViewModels/PowerPreviewViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public PowerPreviewViewModel(ISettingsRepository<PowerPreviewSettings> moduleSet
_monacoMaxFileSize = Settings.Properties.MonacoPreviewMaxFileSize.Value;
_monacoFontSize = Settings.Properties.MonacoPreviewFontSize.Value;
_monacoStickyScroll = Settings.Properties.MonacoPreviewStickyScroll;
_monacoMinimap = Settings.Properties.MonacoPreviewMinimap;

_pdfRenderEnabledGpoRuleConfiguration = GPOWrapper.GetConfiguredPdfPreviewEnabledValue();
if (_pdfRenderEnabledGpoRuleConfiguration == GpoRuleConfigured.Disabled || _pdfRenderEnabledGpoRuleConfiguration == GpoRuleConfigured.Enabled)
Expand Down Expand Up @@ -236,6 +237,7 @@ public PowerPreviewViewModel(ISettingsRepository<PowerPreviewSettings> moduleSet
private int _monacoMaxFileSize;
private bool _monacoStickyScroll;
private int _monacoFontSize;
private bool _monacoMinimap;

private GpoRuleConfigured _pdfRenderEnabledGpoRuleConfiguration;
private bool _pdfRenderEnabledStateIsGPOConfigured;
Expand Down Expand Up @@ -618,6 +620,20 @@ public bool MonacoPreviewStickyScroll
}
}

public bool MonacoPreviewMinimap
{
get => _monacoMinimap;
set
{
if (_monacoMinimap != value)
{
_monacoMinimap = value;
Settings.Properties.MonacoPreviewMinimap = value;
RaisePropertyChanged();
}
}
}

public int MonacoPreviewFontSize
{
get
Expand Down
Loading