[core][ui] update jetpack compose apis to support the showcase#42734
[core][ui] update jetpack compose apis to support the showcase#42734
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
The Pull Request introduced fingerprint changes against the base commit: a089c97 Fingerprint diff[
{
"op": "changed",
"beforeSource": {
"type": "dir",
"filePath": "../../packages/expo-modules-core",
"reasons": [
"expoAutolinkingIos",
"expoAutolinkingAndroid",
"expoAutolinkingIos"
],
"hash": "9e80db6fd0fe039df0d3b2d9fb2c25c454ff20ba"
},
"afterSource": {
"type": "dir",
"filePath": "../../packages/expo-modules-core",
"reasons": [
"expoAutolinkingIos",
"expoAutolinkingAndroid",
"expoAutolinkingIos"
],
"hash": "5f49b983bc0e2a9be3208b27936882877bc34ce0"
}
},
{
"op": "changed",
"beforeSource": {
"type": "dir",
"filePath": "../../packages/expo-ui/android",
"reasons": [
"expoAutolinkingAndroid"
],
"hash": "71d911e821ffd35179576dca47381d2033001f6e"
},
"afterSource": {
"type": "dir",
"filePath": "../../packages/expo-ui/android",
"reasons": [
"expoAutolinkingAndroid"
],
"hash": "87fb316fee431b5fc82327b27ee5735df86ceb2d"
}
}
]Generated by PR labeler 🤖 |
d3212c3 to
ba0e240
Compare
1cb8820 to
eb47ce5
Compare
|
Subscribed to pull request
Generated by CodeMention |
| var drawable by remember { mutableStateOf<Drawable?>(null) } | ||
|
|
||
| // Load icon from URI asynchronously | ||
| LaunchedEffect(source) { |
There was a problem hiding this comment.
nit
| LaunchedEffect(source) { | |
| val uriString = source?.let { resolveUri(it) } | |
| // Load icon from URI asynchronously | |
| LaunchedEffect(uriString) { |
There was a problem hiding this comment.
i would lean toward to keep LaunchedEffect(source) here. it reflects the fact of props.source. if people intentionally pass the same { uri }, the recompose is fine though.
packages/expo-ui/android/src/main/java/expo/modules/ui/icon/VectorIconLoader.kt
Show resolved
Hide resolved
| } | ||
|
|
||
| when (props.variant) { | ||
| "elevated" -> { |
There was a problem hiding this comment.
Are we going to split them into separate components ElevatedCard, OutlinedCard in future?
There was a problem hiding this comment.
yes, will leave it for future work. we still have many variant views somewhere else
| expanded = false, | ||
| onExpandedChange = {}, | ||
| inputField = @Composable { | ||
| SearchBarDefaults.InputField( |
There was a problem hiding this comment.
nit: Later maybe we can export it to JS as a compound component DockedSearchBar.InputField
There was a problem hiding this comment.
leaving for future work. we need to find a way to expose SearchBarDefaults.InputField or other custom input field. may need to find a better way to support this use case
| let leadingIconContent: React.ReactNode = null; | ||
| const regularChildren: React.ReactNode[] = []; | ||
|
|
||
| Children.forEach(props.children as any, (child) => { |
There was a problem hiding this comment.
nit: passing children as it is to native might be better. can help if there's a react component in between in the hierarchy. e.g. Fragment, Suspense or a component defined by user.
For example, we could do below with current setup:
export const DockedSearchBarNativeView.Placeholder = ({children}) => <SlotNativeView slotName="placeholder">{children}</SlotNativeView>There was a problem hiding this comment.
that's a great suggestion. added in the latest commit
intergalacticspacehighway
left a comment
There was a problem hiding this comment.
👏 . Added some comments/questions. Can be done in separate PR too.
packages/expo-ui/android/src/main/java/expo/modules/ui/icon/IconView.kt
Outdated
Show resolved
Hide resolved
eb47ce5 to
410ddc3
Compare
ba0e240 to
fb3baa9
Compare
much appreciated for the great review. updating changes or leaving inline comments. thanks! |
410ddc3 to
96e077d
Compare
fb3baa9 to
baa1223
Compare
96e077d to
bff1687
Compare
baa1223 to
cdb81aa
Compare
bff1687 to
a18b66f
Compare
--------- Co-authored-by: nishan (o^▽^o) <nishanbende@gmail.com>
a18b66f to
d689237
Compare
…42734) # Why add jetpack compose apis that to support the wiki reader show case # How - [core][ui] ad RNHostView as swift-ui - [core] add `nestedScrollConnection` composableScope that allows descendants to access it - [core] add Children with filter support. that to support SlotView for compound components - [core] add `onGlobalEvent` as default registered event. this aligns with swift-ui implementation and to support `modifier.clickable(() => { ... })` - [core] `RecordTypeConverter` to support convertFromMap with nested map - [ui] add Icon which supports loading material symbol xml, e.g. `<Icon source={require('./path/to/symbol.xml'} />`. that to align modern material symbol usage. - [ui] add BasicAlertDialog (m3 api), Card, FlowRow, DockedSearchBar, SearchBar, FilterChip (for aligned naming), HorizontalFloatingToolbar, LazyColumn, ListItem, PullToRefreshBox, RadioButton (for aligned naming), Spacer, Surface, ToggleButton - [ui] add modifiers: width, height, wrapContentWidth, wrapContentHeight, align, clickable, selectable - [ui] rename BottomSheetView to ModalBottomSheet that to align compose api naming - [ui] change clip modifier usage. remove the coupling from Shape component and use predefined constants directly. e.g. `clip(Shapes.RoundedCorner(16))` - [ui] support `<Switch.ThumbContent>` - [ui] add broder style support for Text --------- Co-authored-by: nishan (o^▽^o) <nishanbende@gmail.com>

Why
add jetpack compose apis that to support the wiki reader show case
How
nestedScrollConnectioncomposableScope that allows descendants to access itonGlobalEventas default registered event. this aligns with swift-ui implementation and to supportmodifier.clickable(() => { ... })RecordTypeConverterto support convertFromMap with nested map<Icon source={require('./path/to/symbol.xml'} />. that to align modern material symbol usage.clip(Shapes.RoundedCorner(16))<Switch.ThumbContent>Test Plan
in upstack showcase
Checklist
changelog.mdentry and rebuilt the package sources according to this short guidenpx expo prebuild& EAS Build (eg: updated a module plugin).