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.

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

[Quick Accent] fix showing selector window #22435

Merged
merged 4 commits into from
Dec 4, 2022

Conversation

taras-janea
Copy link
Contributor

@taras-janea taras-janea commented Dec 3, 2022

Summary of the Pull Request

Fix the displaying selector window after the user login.

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Scenario #1

  1. Log-in using userA
  2. Run QuickAccent and insert some letter
  3. Log-in using userB
  4. Log-in using userA
  5. Run QuickAccent and insert some letter

Scenario #2

  1. Log-in via RDP
  2. Run QuickAccent and insert some letter
  3. Close RDP connection, keep session alive
  4. Connect again
  5. Run QuickAccent and insert some letter

In both these scenarios 0.64 version shows a blank rectangle.
The latest version from the main (with improved UI) is not showing a window at all.

@taras-janea taras-janea self-assigned this Dec 3, 2022
@taras-janea taras-janea added the Needs-Review This Pull Request awaits the review of a maintainer. label Dec 3, 2022
@crutkas crutkas mentioned this pull request Dec 3, 2022
10 tasks
@damienleroy
Copy link
Collaborator

I can't believe the solution was so simple... Good catch!
LGTM

@jaimecbernardo
Copy link
Collaborator

I've added some fixes here.
The problem with calculating the position when the window has been hidden means that it won't have the correct size. Because of this I had some accents clipped from view when it's on the bottom right, for example.

Not related to this PR and also present in main was the issue that offsets were not being applied in the same way in left vs right or top vs bottom positions. The reason for this being that margins were added to the base grid and then a offset applied. This made our offset calculation make little sense, so I've removed the margins and added a little offset to make it look similar.

Copy link
Collaborator

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

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

LGTM! Great work. I've applied some fixes on top of it and will merge soon.

@jaimecbernardo jaimecbernardo merged commit 3afa124 into microsoft:main Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Review This Pull Request awaits the review of a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants