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

Feat: Bring Window to Foreground on Login #1544

Merged
merged 9 commits into from
Jun 5, 2024
Merged

Feat: Bring Window to Foreground on Login #1544

merged 9 commits into from
Jun 5, 2024

Conversation

Sewer56
Copy link
Member

@Sewer56 Sewer56 commented Jun 4, 2024

This is a basic implementation for:

Although this PR is fairly simple, and adheres to UI Guidelines... it feels, weird. For some reason I don't feel good about it.
If you have any better ideas for implementing this, please let me know.

I could integrate this sort of functionality into the workspace system, and persist the window that started the login request in a global location so the correct one is brought to front. However, I've decided to keep things as simple as possible for now. Let me know how far you want me to go with this.

@Sewer56 Sewer56 added the Design UI/UX This is related to the UI. label Jun 4, 2024
@Sewer56 Sewer56 requested a review from a team June 4, 2024 20:48
@Sewer56 Sewer56 self-assigned this Jun 4, 2024
@erri120 erri120 self-requested a review June 4, 2024 20:54
src/NexusMods.App.UI/Windows/IMainWindowViewModel.cs Outdated Show resolved Hide resolved
src/NexusMods.App.UI/Windows/MainWindow.axaml.cs Outdated Show resolved Hide resolved
src/NexusMods.App.UI/Windows/MainWindow.axaml.cs Outdated Show resolved Hide resolved
src/NexusMods.App.UI/Windows/MainWindowViewModel.cs Outdated Show resolved Hide resolved
@erri120
Copy link
Member

erri120 commented Jun 5, 2024

@Al12rs can you test this on Windows? I can't test this on Linux.

@Al12rs
Copy link
Contributor

Al12rs commented Jun 5, 2024

@Al12rs can you test this on Windows? I can't test this on Linux.

I already tested this on Windows and with the fix on the name it does work.
Edit:
If the App window is not minimized it does bring the window on top. If it is minimized nothing happens.
I was hoping it would flash the icon in the windows task bar at least.

@Sewer56
Copy link
Member Author

Sewer56 commented Jun 5, 2024

I got the feedback sorted, hopefully should be good now.


Extra Note:

I tested this on both Linux and Windows. Though on Linux I normally use a tiling WM so my window is already topmost, so I had to install an alternative Window Manager to test this.

I only initially brought it to focus if it wasn't minimized, because it's what the ticket requested. In a normal UX flow, the user opens the browser from the App, and the App fills the screen. I did make it unminimize the window now though.

@Al12rs Al12rs self-requested a review June 5, 2024 13:42
@Al12rs Al12rs merged commit 605152d into main Jun 5, 2024
2 of 6 checks passed
@Al12rs Al12rs deleted the window-foreground branch June 5, 2024 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design UI/UX This is related to the UI.
3 participants