Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
JetStream and Catalog updates #165
JetStream and Catalog updates #165
Changes from 13 commits
cc17bb0
d3ca67c
d82d404
d87a583
078ab77
9fca22a
70bfda9
1f3ef36
3650f67
34dd960
dd1eb7c
0859aeb
e9a2983
cd2bc04
ca73185
d76249b
0474829
284ec30
e94e1a9
b9e797d
aab62e2
8c68712
ddaa53b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
nit: redundant import
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.
I don't think we need this. If the item gains focus, it automatically gets saved if the parent grid is wrapped with "focusRestorer"
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.
This could lead to crashes because of race condition. It is possible that the LaunchedEffect is executed before the items are placed and it would cause a crash. We can make use of
onPlaced
oronGloballyPositioned
modifiers to see if the item is available to get focus. Refer b/276738340#comment6There 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 don't need to make use of
focusRequester
modifier to save and restore focus when usingfocusRestorer
modifier.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.
Removing this gives me an exception:
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.
The
focusRequester
modifier is necessary as therequest
focus method is called over the associatedFocusRequester
object. Without the modifier, callingrequestFocus
method crashes the app.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.
Actually, focusRestorer doesn't need any focusRequester modifier, to work. If the app is crashing, it could be a bug, most likely it would be because of the 3rd point: "attempting to request focus during composition".
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.
I just noticed that there is a Launched Effect which is requesting focus on the focus requester. If the focusRequester is not initialized, it will crash the app. I think the goal here is to request focus on the first element. In that case, the focusRequester should be assigned to the first item's modifier and not the parent grid.