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

Jump to bottom

[Preview Pane] Sticky scroll setting is handled twice #33288

Open
PesBandi opened this issue Jun 7, 2024 · 5 comments · Fixed by #33384
Open

[Preview Pane] Sticky scroll setting is handled twice #33288

PesBandi opened this issue Jun 7, 2024 · 5 comments · Fixed by #33384
Labels
Cost-Small Small work item - 0-8 hours of work Good first issue Good for newcomers. Issue-Bug Something isn't working Product-File Explorer Power Toys that touch explorer like Preview Pane Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
PowerToys 0.82

Comments

@PesBandi
Copy link
Contributor

PesBandi commented Jun 7, 2024

Microsoft PowerToys version

v0.81

Installation method

Other (please specify in "Steps to Reproduce")

Running as admin

None

Area(s) with issue?

File Explorer: Preview Pane

Steps to reproduce

Lines 400 and 403 are exactly the same thing. There's no need to replace PT_STICKY_SCROLL twice.

PowerToys/src/modules/previewpane/MonacoPreviewHandler/MonacoPreviewHandlerControl.cs

Lines 395 to 404 in ed249bc

// 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_THEME]]", Settings.GetTheme(), StringComparison.InvariantCulture);
_html = _html.Replace("[[PT_STICKY_SCROLL]]", _settings.StickyScroll ? "1" : "0", 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_STICKY_SCROLL]]", _settings.StickyScroll ? "1" : "0", StringComparison.InvariantCulture);
_html = _html.Replace("[[PT_URL]]", FilePreviewCommon.MonacoHelper.VirtualHostName, StringComparison.InvariantCulture);

✔️ Expected Behavior

No response

❌ Actual Behavior

No response

Other Software

No response
@PesBandi PesBandi added Issue-Bug Something isn't working Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams labels Jun 7, 2024
@htcfreek htcfreek added Product-File Explorer Power Toys that touch explorer like Preview Pane Product-Peek Refers to Peek Powertoys and removed Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams labels Jun 7, 2024
@htcfreek
Copy link
Collaborator

htcfreek commented Jun 7, 2024

@Aaron-Junker
Correct me if I am wrong. But don't we apply the settings on two places in the code? If yes, we should check the other place too.

@PesBandi
Copy link
Contributor Author

PesBandi commented Jun 7, 2024

@htcfreek I checked it. It is OK in Peek.

@htcfreek htcfreek removed the Product-Peek Refers to Peek Powertoys label Jun 7, 2024
@Jay-o-Way
Copy link
Collaborator

Nit that can be added to pr: en-us/ line 409

@htcfreek
Copy link
Collaborator

htcfreek commented Jun 9, 2024

Nit that can be added to pr: en-us/ line 409

nice catch.

@htcfreek htcfreek added Good first issue Good for newcomers. Cost-Small Small work item - 0-8 hours of work labels Jun 11, 2024
downarowiczd added a commit to downarowiczd/33288-PowerToys-StickyScroll that referenced this issue Jun 14, 2024
@downarowiczd
[Preview Pane] Sticky scroll setting is handled twice microsoft#33288
412770d
@downarowiczd downarowiczd mentioned this issue Jun 14, 2024
Merged
@htcfreek htcfreek added the Status-In progress This issue or work-item is under development label Jun 15, 2024
@jaimecbernardo jaimecbernardo added Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed Status-In progress This issue or work-item is under development labels Jun 17, 2024
@jaimecbernardo jaimecbernardo added this to the PowerToys 0.82 milestone Jun 17, 2024
@htcfreek
Copy link
Collaborator

@jaimecbernardo
Plaese reopen. Issue was auto closed on merge.

@Aaron-Junker Aaron-Junker reopened this Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Cost-Small Small work item - 0-8 hours of work Good first issue Good for newcomers. Issue-Bug Something isn't working Product-File Explorer Power Toys that touch explorer like Preview Pane Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
PowerToys 0.82
5 participants
@jaimecbernardo @Aaron-Junker @htcfreek @Jay-o-Way @PesBandi