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

[dartpad_ui] Add splash screen #3089

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

cheymos
Copy link

@cheymos cheymos commented Nov 9, 2024

Added a simple splash screen so that when you open dartpad.dev you don't have to look at a white screen. 🙂
Implemented with mobile devices and theme change in mind.

Mobile Demo
Screen.Recording.2024-11-09.at.21.27.45.mov
Desktop Demo
Screen.Recording.2024-11-09.at.21.25.07.mov

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

Copy link

google-cla bot commented Nov 9, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@cheymos
Copy link
Author

cheymos commented Nov 9, 2024

@parlough Hi! What do you think about this PR?

Copy link

github-actions bot commented Nov 9, 2024

PR Health

Breaking changes ✔️
Package Change Current Version New Version Needed Version Looking good?
Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

Coverage ✔️
File Coverage

This check for test coverage is informational (issues shown here will not fail the PR).

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package Leaked API symbols
License Headers ✔️
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers
Files
pkgs/samples/lib/brick_breaker.dart
pkgs/samples/lib/default_dart.dart
pkgs/samples/lib/default_flutter.dart
pkgs/samples/lib/fibonacci.dart
pkgs/samples/lib/google_ai.dart
pkgs/samples/lib/hello_world.dart
pkgs/samples/lib/main.dart
pkgs/samples/lib/sunflower.dart
pkgs/samples/tool/samples.dart

@cheymos
Copy link
Author

cheymos commented Nov 15, 2024

@devoncarew Hi! I'm sorry if I'm tagging you. But could you please tell me how I should deal with this PR?

Copy link
Member

@devoncarew devoncarew left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I like that you've moved the index.html styles into a new file - I think that helps with readability.

Wrt the white flash at startup, I do see it now myself; I expect that it's noticeable when using the dark theme. We probably don't need the dart icon as part of the splash screen, and, we may want to make the exit animation for the splash screen shorter as it would otherwise delay visibility by a 1/2 sec.

You may generally want to start with an issue instead of a PR for feature contributions - it would help save you time if we decide against the feature in general. But I'm probably not the right reviewer; I'll bump you over to @johnpryan and @parlough.

}

#splash.light {
background-color: #f5f5f7;
Copy link
Member

Choose a reason for hiding this comment

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

I assume this color is the color we'd settle into after we've started up in the light theme?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, that's right.

}

#splash.dark {
background-color: #1f2833;
Copy link
Member

Choose a reason for hiding this comment

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

Same with this with the dark theme?

Copy link
Author

Choose a reason for hiding this comment

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

Yep.

@cheymos
Copy link
Author

cheymos commented Nov 15, 2024

Thanks for the contribution! I like that you've moved the index.html styles into a new file - I think that helps with readability.

Wrt the white flash at startup, I do see it now myself; I expect that it's noticeable when using the dark theme. We probably don't need the dart icon as part of the splash screen, and, we may want to make the exit animation for the splash screen shorter as it would otherwise delay visibility by a 1/2 sec.

You may generally want to start with an issue instead of a PR for feature contributions - it would help save you time if we decide against the feature in general. But I'm probably not the right reviewer; I'll bump you over to @johnpryan and @parlough.

You're right, I was a bit rushed with the PR though. I'll start with issues next time.
These changes didn't take me long to make, so I could redo them if needed.

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