Skip to content

Conversation

onesounds
Copy link
Contributor

@onesounds onesounds commented Apr 10, 2025

What's the PR

  • Make Result badge great again.
  • Add BadgeIcoPath string & BadgeIcon icon delegate & ShowBadge bool property in Result class so that plugins can change the results of their badges.
  • Fix global search determination issue in MainViewModel. (if (query.ActionKeyword == Plugin.Query.GlobalPluginWildcardSign))

Related #2721. Close #2517.

Test

  • Setting the result badge

image

  • Show the result badge

image

This comment has been minimized.

@Jack251970 Jack251970 mentioned this pull request Apr 10, 2025
1 task

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@Jack251970 Jack251970 changed the title Plugin Badge Result Badge Apr 11, 2025
@prlabeler prlabeler bot added the bug Something isn't working label Apr 11, 2025
@Jack251970 Jack251970 added the enhancement New feature or request label Apr 11, 2025
@Jack251970 Jack251970 requested a review from Copilot April 11, 2025 08:20
@Jack251970 Jack251970 added this to the 1.20.0 milestone Apr 11, 2025
@Jack251970 Jack251970 added the kind/ui related to UI, icons, themes, etc label Apr 11, 2025
- Fix glyph margin in win11light

This comment has been minimized.

@onesounds
Copy link
Contributor Author

onesounds commented Apr 11, 2025

🛠️ Changes







  • The badge icon was originally positioned at the bottom-right corner of the main icon. However, this layout would break when the font size or other layout properties changed.

  • To solve this, the badge was changed to a larger square image that overlays the main icon. The developer can manually adjust the badge’s position within its transparent PNG.

  • @Jack251970 I'll upload the PNG to the team chat. Need to add it to the Windows Worker plugin, apply it, and test it.

💬 Notes

  • Ideally, the badge area would be slightly larger than the main icon so it could appear offset further toward the bottom-right, which would look better visually.
    However, this would require an additional size converter — and honestly, that felt like too much hassle — so we went with the simpler solution.
    As a result, the badge can't be pushed further out beyond the icon’s bounds for now.

  • There was some consideration about using the badge for other purposes in the future. If needed, we’ll just add another overlay layer with a different name.
    For now, this “badge” is dedicated solely to this specific use case.

  • Previously, the glyph and icon positions were slightly misaligned. This change ensures they are now perfectly aligned.
    Additionally, the glyph position was slightly off in the Windows 11 theme, so it was adjusted accordingly.

@Jack251970 Jack251970 requested a review from Copilot April 12, 2025 06:45
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 9 changed files in this pull request and generated 1 comment.

Files not reviewed (4)
  • Flow.Launcher/Languages/en.xaml: Language not supported
  • Flow.Launcher/ResultListBox.xaml: Language not supported
  • Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml: Language not supported
  • Flow.Launcher/Themes/Win11Light.xaml: Language not supported
Comments suppressed due to low confidence (3)

Flow.Launcher/ViewModel/MainViewModel.cs:1211

  • Changing the global query check from an explicit value (GlobalPluginWildcardSign) to a string null/empty check may alter query classification. Please verify that this condition correctly distinguishes global queries without affecting intended local queries.
if (string.IsNullOrEmpty(query.ActionKeyword))

Flow.Launcher/ViewModel/ResultViewModel.cs:127

  • [nitpick] The new ShowBadge property uses a global query determination (IsGlobalQuery) and badge settings to decide visibility. Confirm that this logic aligns with user expectations and that the conditions correctly reflect when badges should be shown.
public Visibility ShowBadge

Flow.Launcher/Plugin/Result.cs:191

  • [nitpick] Updating both IcoPath and BadgeIcoPath when PluginDirectory is set ensures absolute paths for icon loading, but double-check that this behavior does not unintentionally override custom paths previously set.
PluginDirectory setter updating IcoPath and BadgeIcoPath

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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/Converters/BadgePositionConverter.cs (1)

1-32: Consider improving constant handling and simplifying conditional logic.

The converter computes badge positions using hard-coded constants (8 and 2) which would benefit from explicit naming for clarity and maintainability. Additionally, the identical logic for both X-Offset and Y-Offset parameters (1 and 2) suggests potential for simplification.

public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
{
    if (value is double actualWidth && parameter is string param)
    {
-        double offset = actualWidth / 2 - 8;
+        const double BadgeRadius = 8;
+        const double AdditionalOffset = 2;
+        double offset = actualWidth / 2 - BadgeRadius;

-        if (param == "1") // X-Offset
-        {
-            return offset + 2;
-        }
-        else if (param == "2") // Y-Offset
-        {
-            return offset + 2;
-        }
+        if (param == "1" || param == "2") // X-Offset or Y-Offset
+        {
+            return offset + AdditionalOffset;
+        }
    }

    return 0.0;
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 02b5373 and 5d39501.

📒 Files selected for processing (3)
  • Flow.Launcher/Converters/BadgePositionConverter.cs (1 hunks)
  • Flow.Launcher/Converters/SizeRatioConverter.cs (1 hunks)
  • Flow.Launcher/ResultListBox.xaml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Flow.Launcher/ResultListBox.xaml
🔇 Additional comments (1)
Flow.Launcher/Converters/SizeRatioConverter.cs (1)

1-27: Great implementation for size scaling with consistent culture handling.

The converter properly handles culture-independent parsing and has good error handling. The implementation will correctly scale dimensions based on the provided ratio parameter.

@onesounds
Copy link
Contributor Author

  • A position and size converter has been added (bottom-right). The badge icon is now positioned at the bottom-right with a scale of 0.6.

  • If you simply want to use the plugin icon as the badge icon, there's no need to create a separate image.

  • The badge is now positioned relative to the icon, so its position remains consistent even if the layout changes. Its size also scales proportionally. (You can test this with the slimlight theme.)

This comment has been minimized.

@Jack251970 Jack251970 requested a review from Copilot April 12, 2025 12:23
@Jack251970 Jack251970 removed the bug Something isn't working label Apr 12, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 7 out of 11 changed files in this pull request and generated no comments.

Files not reviewed (4)
  • Flow.Launcher/Languages/en.xaml: Language not supported
  • Flow.Launcher/ResultListBox.xaml: Language not supported
  • Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml: Language not supported
  • Flow.Launcher/Themes/Win11Light.xaml: Language not supported

Copy link

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

❌ Errors Count
❌ forbidden-pattern 22
⚠️ non-alpha-in-dictionary 19

See ❌ Event descriptions for more information.

If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

@prlabeler prlabeler bot added the bug Something isn't working label Apr 12, 2025
@onesounds onesounds merged commit 9b5d22a into Flow-Launcher:dev Apr 12, 2025
4 checks passed
@Jack251970 Jack251970 added Documentation Update required to documentation and removed bug Something isn't working labels Apr 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Update required to documentation enhancement New feature or request kind/ui related to UI, icons, themes, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add small icon identifier to Window Walker results
2 participants