-
-
Notifications
You must be signed in to change notification settings - Fork 334
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.
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
Fix main window resize border thickness issue #3232
base: dev
Are you sure you want to change the base?
Conversation
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
🥷 Code experts: onesounds Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThe changes update how the application handles window chrome styling. In the Theme class, a new using directive is added and the AddDropShadowEffectToCurrentTheme method now uses a variable for margin definition and calls a new SetResizeBoarderThickness method, which computes the resize border thickness based on a provided margin. The RemoveDropShadowEffectFromCurrentTheme method similarly calls this new method with null to reset the border thickness. Additionally, the MainWindow.xaml file is modified by removing the ResizeBorderThickness attribute from the WindowChrome element. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant T as Theme
participant W as WindowChrome
U->>T: Invoke AddDropShadowEffectToCurrentTheme
T->>T: Define 'margin' (Thickness)
T->>T: Call SetResizeBoarderThickness(margin)
T->>W: Update ResizeBorderThickness property
sequenceDiagram
participant U as User
participant T as Theme
participant W as WindowChrome
U->>T: Invoke RemoveDropShadowEffectFromCurrentTheme
T->>T: Call SetResizeBoarderThickness(null)
T->>W: Reset ResizeBorderThickness to system default
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher.Core/Resource/Theme.cs (1)
366-389
: Fix typo in method name: "Boarder" should be "Border".The implementation correctly handles the resize border thickness calculation, but there's a typo in the method name.
Apply this diff to fix the typo:
- private static void SetResizeBoarderThickness(Thickness? effectMargin) + private static void SetResizeBorderThickness(Thickness? effectMargin)Don't forget to update the method calls at lines 318 and 361 as well:
- SetResizeBoarderThickness(margin); + SetResizeBorderThickness(margin);- SetResizeBoarderThickness(null); + SetResizeBorderThickness(null);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher.Core/Resource/Theme.cs
(4 hunks)Flow.Launcher/MainWindow.xaml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (4)
Flow.Launcher.Core/Resource/Theme.cs (3)
14-14
: LGTM!The added using directive is correctly placed and necessary for accessing the
WindowChrome
class.
310-318
: LGTM! The changes improve readability and fix the resize border issue.The margin variable extraction and the call to
SetResizeBoarderThickness
are well-placed to ensure the resize border thickness is correctly set when adding the drop shadow effect.
361-361
: LGTM! The changes ensure proper cleanup of resize border thickness.The call to
SetResizeBoarderThickness(null)
correctly resets the resize border thickness when removing the drop shadow effect.Flow.Launcher/MainWindow.xaml (1)
40-40
: LGTM! The removal of ResizeBorderThickness is correct.The resize border thickness is now managed dynamically by the Theme class through the
SetResizeBorderThickness
method, making this static attribute unnecessary.
Issue
Because adding drop shadow effect will change the margin of the window border, we need to update the resize thickness of the window chrome to correct set the resize border. Otherwise, scrollbar selection is unable to work because resize area override it.
Query window shadow effect
breaks scrollbar selection #3185Tested Video:
2025-02-07.20-22-33.mp4