Skip to content

Conversation

dcog989
Copy link
Contributor

@dcog989 dcog989 commented Sep 26, 2025

Closes #3917

@github-actions github-actions bot added this to the 2.1.0 milestone Sep 26, 2025
Copy link

@check-spelling-bot Report

🔴 Please review

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

❌ Errors and Warnings Count
❌ forbidden-pattern 2
⚠️ non-alpha-in-dictionary 2

See ❌ Event descriptions for more information.

Forbidden patterns 🙅 (1)

In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves.

These forbidden patterns matched content:

s.b. workaround(s)

\bwork[- ]arounds?\b
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.

Copy link

gitstream-cm bot commented Sep 26, 2025

🥷 Code experts: Jack251970, onesounds

Jack251970, onesounds have most 👩‍💻 activity in the files.
Jack251970, onesounds have most 🧠 knowledge in the files.

See details

Flow.Launcher/MainWindow.xaml.cs

Activity based on git-commit:

Jack251970 onesounds
SEP
AUG 0 additions & 2 deletions
JUL 57 additions & 45 deletions
JUN 91 additions & 71 deletions 87 additions & 18 deletions
MAY 145 additions & 43 deletions 5 additions & 1 deletions
APR 69 additions & 45 deletions 5 additions & 1 deletions

Knowledge based on git-blame:
Jack251970: 64%
onesounds: 15%

✨ Comment /gs review for LinearB AI review. Learn how to automate it here.

Copy link

gitstream-cm bot commented Sep 26, 2025

Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX.

@coderabbitai coderabbitai bot added the bug Something isn't working label Sep 26, 2025
Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

📝 Walkthrough

Walkthrough

Adjusted the VerticalCenter calculation in MainWindow.xaml.cs to base vertical centering on the window’s ActualHeight instead of QueryTextBox.ActualHeight, changing the factor from quarter to half to compute the top position.

Changes

Cohort / File(s) Summary
Window positioning logic
Flow.Launcher/MainWindow.xaml.cs
Updated VerticalCenter top calculation from (dip2.Y - QueryTextBox.ActualHeight) / 4 + dip1.Y to (dip2.Y - ActualHeight) / 2 + dip1.Y to use the window’s ActualHeight for centering.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

bug

Suggested reviewers

  • onesounds
  • taooceros
  • jjw24

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly summarizes that the window will be vertically centered when the “Center” setting is selected, directly reflecting the main functional change implemented in the pull request.
Linked Issues Check ✅ Passed The pull request updates the VerticalCenter calculation in MainWindow.xaml.cs to use the window’s ActualHeight and a 1/2 multiplier, which directly fulfills the objective of vertically centering the window as specified in issue #3917.
Out of Scope Changes Check ✅ Passed The only change in this pull request is the adjustment to the vertical centering formula within MainWindow.xaml.cs, which is directly related to the linked issue and does not introduce any unrelated or out-of-scope modifications.
Description Check ✅ Passed The description directly references the closure of the linked issue #3917 and is related to the changeset that implements the vertical centering fix, satisfying the requirement to be on-topic.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e068104 and 88d1236.

📒 Files selected for processing (1)
  • Flow.Launcher/MainWindow.xaml.cs (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Check Spelling
Flow.Launcher/MainWindow.xaml.cs

[warning] 115-115: Unrecognized spelling: 'Wnd'.


[warning] 372-372: Unrecognized spelling: 'Wnd'.


[warning] 585-585: Unrecognized spelling: 'Wnd'.


[warning] 587-587: Unrecognized spelling: 'Wnd'.


[warning] 774-774: Unrecognized spelling: 'gamemode'.


[warning] 775-775: Unrecognized spelling: 'gamemode'.


[warning] 778-778: Unrecognized spelling: 'gamemode'.


[warning] 781-781: Unrecognized spelling: 'positionreset'.


[warning] 784-784: Unrecognized spelling: 'positionreset'.


[warning] 800-800: Unrecognized spelling: 'gamemode'.


[warning] 801-801: Unrecognized spelling: 'positionreset'.


[warning] 806-806: Unrecognized spelling: 'positionreset'.


[warning] 924-924: Unrecognized spelling: 'XRatio'.


[warning] 925-925: Unrecognized spelling: 'YRatio'.


[warning] 1139-1139: Unrecognized spelling: 'clocksb'.


[warning] 1140-1140: Unrecognized spelling: 'clocksb'.


[warning] 1141-1141: Unrecognized spelling: 'iconsb'.


[warning] 1142-1142: Unrecognized spelling: 'iconsb'.


[warning] 1147-1147: Unrecognized spelling: 'clocksb'.


[warning] 1148-1148: Unrecognized spelling: 'iconsb'.


[warning] 67-67: Unrecognized spelling: 'Wnd'.


[warning] 231-231: Unrecognized spelling: 'activing'.


[warning] 277-277: Unrecognized spelling: 'gamemode'.


[warning] 414-414: Unrecognized spelling: 'mainwindow'.


[warning] 502-502: Unrecognized spelling: 'Arrowkeys'.


[warning] 780-780: Unrecognized spelling: 'positionreset'.


[error] 821-821: work around matches a line_forbidden.patterns entry: \bwork[- ]arounds?\b (forbidden-pattern)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: gitStream.cm
  • GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher/MainWindow.xaml.cs (1)

992-996: Centering now uses the full window height correctly

Using ActualHeight in the centering math finally bases the calculation on the entire window instead of the input box, so the window lands dead-center vertically even under DPI scaling. Looks good.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Member

@Jack251970 Jack251970 left a comment

Choose a reason for hiding this comment

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

There are two bugs here:

  1. When window is shown at the first time. Its position is weird.
  2. When ActualHeight is changed, its position is not updated. And the next time you open the main window, it will stay in the same position as origin. (In demo video, I disabled Home Page feature to change the ActualHeight of main window when query is empty)

Demo video:

2025-09-27.12-54-49.mp4

@VictoriousRaptor
Copy link
Contributor

VictoriousRaptor commented Sep 27, 2025

Please do not change it this way. If we really vertically center it when we have 9 results, the bottom ones may be out of the screen.

We can create a new option that really centers, but please do not change this one.

@dcog989
Copy link
Contributor Author

dcog989 commented Sep 27, 2025

we have 9 results, the bottom ones may be out of the screen.

That is not the result - it centers on ActualHeight so whether we display 1 line or 20, that 'block' is vertically centered on the screen. So that provides what the user would expect when they choose 'Center', as opposed to 'somewhere near the top of the page' with current behavior?

Please clarify then I can address issues that @Jack251970 raises.

@VictoriousRaptor
Copy link
Contributor

we have 9 results, the bottom ones may be out of the screen.

That is not the result - it centers on ActualHeight so whether we display 1 line or 20, that 'block' is vertically centered on the screen. So that provides what the user would expect when they choose 'Center', as opposed to 'somewhere near the top of the page' with current behavior?

Please clarify then I can address issues that @Jack251970 raises.

Well my suggestion is don't change the behavior of the current Center option. Created a new one maybe better for user experience.

@Jack251970
Copy link
Member

I agree. Please do not change the behaviour of this option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: 'Center' for window position is not vertically centered
3 participants