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

Allow splash screen window to be resizable #68

Merged
merged 1 commit into from
Jun 23, 2023

Conversation

sergeichestakov
Copy link
Contributor

Why

It's been one of our most common complaints

What changed

Allow splash screen window to be resizable (still has the same min height/width as before and also doesn't persist height across sessions)

Test plan

  • Open app
  • Resize splash screen window

Copy link
Member

@szymonkaliski szymonkaliski left a comment

Choose a reason for hiding this comment

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

I think we also have to update getSplashScreenWindowBounds() as right now it hardcodes width/height

@sergeichestakov
Copy link
Contributor Author

oh no I'm intentionally not restoring the previous width/height here. see PR description

@szymonkaliski
Copy link
Member

yea but why? I think we should be consistent -- we store position and size of the main window, we should do the same here if we allow resizing

@sergeichestakov
Copy link
Contributor Author

The rational here is that we should always default to this size since it's what we've optimized for in the UI. I'm not sure there would be an expectation that the size here is persisted like with repl windows.

Also feels more ergonomic in the case where you open multiple new windows

@szymonkaliski
Copy link
Member

I'm not sold on this, I think we should persist both the size and the position, or neither of them (so the splash screen always opens in the center of the screen with the default size, which I also like).
But, this also feels like a bikeshed, so if you're convinced this is the way to go feel free to merge :)

@sergeichestakov
Copy link
Contributor Author

I think we should persist both the size and the position, or neither of them

hmm fair but we only persist the later now. didn't want to change that behavior in the context of this change since that's a separate debate. but yeah let's try this and see how it feels then we can follow up

@sergeichestakov sergeichestakov merged commit 6e5ccda into main Jun 23, 2023
@sergeichestakov sergeichestakov deleted the @sergeichestakov/splash-screen-resizable branch June 23, 2023 16:11
sergeichestakov added a commit that referenced this pull request Jul 10, 2023
sergeichestakov added a commit that referenced this pull request Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants