-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Preserve state on SDK switch (#22430) #22735
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
Preserve state on SDK switch (#22430) #22735
Conversation
# Conflicts: # playground/frontend/lib/pages/playground/states/playground_state.dart
|
|
||
| set source(String source) { | ||
| codeController.text = source; | ||
| void setSource(String source) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We recently turned this from setSource to set source following the linter, but now we need nullable getter with a non-nullable setter, and that does not work, so back to the method again.
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
| await Future.wait(SDK.values.map(_loadDefaultExample)); | ||
| } catch (ex) { | ||
| if (defaultExamplesMap.isEmpty) { | ||
| rethrow; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used to load the 4 default examples sequentially, and if any of them failed, nothing was loaded. Now go in parallel and allow failures in some of them.
| final controller = state.snippetEditingController; | ||
|
|
||
| if (controller == null) { | ||
| return const LoadingIndicator(size: kLgLoadingIndicatorSize); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used to show a blank editor before the example is loaded, and it was confusing. Now we only get the controller with the example already in it, unless deliberately showing an empty editor.
damondouglas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pabloem LGTM
|
LGTM thanks |
Resolves #22430
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.